Make sure dSYMs are created by default for Mac sanitizer builds.

Make sure dSYMs are created by default for Mac sanitizer builds.

They should be created by default for sanitizer builds (and were w/
GYP), but that setting got lost in the GN migration.

Also, this CL corrects the default logic for enable_dsyms and
enable_stripping so that we only do this for internal official
builds by default, rather than for all "official" builds (
since is_official_build is really an optimization setting, not
a description of how things should be packaged).

R=rsesek@chromium.org, thakis@chromium.org, inferno@chromium.org
BUG=635242
Committed: https://crrev.com/8b0cda7f8d3f9a4f1d53c78600fb0c62c7eec7f9
Cr-Commit-Position: refs/heads/master@{#410979}

[Dirk Pranke, 2016-08-10 01:59:53]

https://codereview.chromium.org/2230723002/diff/1/build/config/mac/symbols.gni
File build/config/mac/symbols.gni (right):

https://codereview.chromium.org/2230723002/diff/1/build/config/mac/symbols.gn...
build/config/mac/symbols.gni:17: enable_dsyms = (is_official_build &&
is_chrome_branded) || is_using_sanitizer
assuming we don't actually want these generated for all official, but not
chrome_branded, builds, of course.

[Dirk Pranke, 2016-08-10 01:59:57]

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

[commit-bot: I haz the power, 2016-08-10 02:00:35]

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-08-10 02:05:26]

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

[commit-bot: I haz the power, 2016-08-10 02:05:26]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[inferno, 2016-08-10 04:09:02]

The CQ bit was checked by inferno@chromium.org

[inferno, 2016-08-10 04:09:02]

lgtm

Thanks for fixing this for sanitizer in gn world.

[commit-bot: I haz the power, 2016-08-10 04:09:29]

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-10 06:19:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-10 06:21:23]

Description was changed from

==========
Make sure dSYMs are created by default for Mac sanitizer builds.

They should be created by default for sanitizer builds (and were w/
GYP), but that setting got lost in the GN migration.

Also, this CL corrects the default logic for enable_dsyms and
enable_stripping so that we only do this for internal official
builds by default, rather than for all "official" builds (
since is_official_build is really an optimization setting, not
a description of how things should be packaged).

R=rsesek@chromium.org, thakis@chromium.org, inferno@chromium.org
BUG=635242
==========

to

==========
Make sure dSYMs are created by default for Mac sanitizer builds.

They should be created by default for sanitizer builds (and were w/
GYP), but that setting got lost in the GN migration.

Also, this CL corrects the default logic for enable_dsyms and
enable_stripping so that we only do this for internal official
builds by default, rather than for all "official" builds (
since is_official_build is really an optimization setting, not
a description of how things should be packaged).

R=rsesek@chromium.org, thakis@chromium.org, inferno@chromium.org
BUG=635242

Committed: https://crrev.com/8b0cda7f8d3f9a4f1d53c78600fb0c62c7eec7f9
Cr-Commit-Position: refs/heads/master@{#410979}
==========

[commit-bot: I haz the power, 2016-08-10 06:21:24]

Patchset 2 (id:??) landed as
https://crrev.com/8b0cda7f8d3f9a4f1d53c78600fb0c62c7eec7f9
Cr-Commit-Position: refs/heads/master@{#410979}

[Robert Sesek, 2016-08-10 14:45:42]

not lgtm

I don't agree with the change made to enable_dsyms. Stripping symbols greatly
reduces the size of the binary, which makes it faster, so it very much is a part
of is_official_build IMO.

[Dirk Pranke, 2016-08-10 16:34:19]

On 2016/08/10 14:45:42, Robert Sesek wrote:
> not lgtm
> 
> I don't agree with the change made to enable_dsyms. Stripping symbols greatly
> reduces the size of the binary, which makes it faster, so it very much is a
part
> of is_official_build IMO.

This is why I wanted you to review this, but I think inferno@ checked the commit
box :).

I don't have a strong leaning either way, and this seems like a good reason to
me. I'll
post a follow-up to flip it back.

[aarya, 2016-08-10 17:00:11]

On 2016/08/10 16:34:19, Dirk Pranke wrote:
> On 2016/08/10 14:45:42, Robert Sesek wrote:
> > not lgtm
> > 
> > I don't agree with the change made to enable_dsyms. Stripping symbols
greatly
> > reduces the size of the binary, which makes it faster, so it very much is a
> part
> > of is_official_build IMO.
> 
> This is why I wanted you to review this, but I think inferno@ checked the
commit
> box :).
> 

Sorry my bad, i shouldn't have checked the cq button.

> I don't have a strong leaning either way, and this seems like a good reason to
> me. I'll
> post a follow-up to flip it back.

[Robert Sesek, 2016-08-10 21:42:09]

On 2016/08/10 16:34:19, Dirk Pranke wrote:
> On 2016/08/10 14:45:42, Robert Sesek wrote:
> > not lgtm
> > 
> > I don't agree with the change made to enable_dsyms. Stripping symbols
greatly
> > reduces the size of the binary, which makes it faster, so it very much is a
> part
> > of is_official_build IMO.
> 
> This is why I wanted you to review this, but I think inferno@ checked the
commit
> box :).
> 
> I don't have a strong leaning either way, and this seems like a good reason to
> me. I'll
> post a follow-up to flip it back.

Did this happen yet?

[Robert Sesek, 2016-08-10 23:09:45]

On 2016/08/10 21:42:09, Robert Sesek wrote:
> On 2016/08/10 16:34:19, Dirk Pranke wrote:
> > On 2016/08/10 14:45:42, Robert Sesek wrote:
> > > not lgtm
> > > 
> > > I don't agree with the change made to enable_dsyms. Stripping symbols
> greatly
> > > reduces the size of the binary, which makes it faster, so it very much is
a
> > part
> > > of is_official_build IMO.
> > 
> > This is why I wanted you to review this, but I think inferno@ checked the
> commit
> > box :).
> > 
> > I don't have a strong leaning either way, and this seems like a good reason
to
> > me. I'll
> > post a follow-up to flip it back.
> 
> Did this happen yet?

https://codereview.chromium.org/2232943003