Move default sanitizer options into build/ so that WebRTC can import them.

Move default sanitizer options into build/ so that WebRTC can import them.

BUG=302040
TBR=noelallen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291168

[kjellander_chromium, 2014-06-25 12:27:12]

https://codereview.chromium.org/357623003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/357623003/diff/1/build/common.gypi#newcode3885
build/common.gypi:3885: 'dependencies': [
I think you should do something like:
'includes': ['<(DEPTH)/build/sanitizers/sanitizers.gypi']
'dependencies': ['sanitizer_options']
instead (not sure exactly that works, but you get the idea).

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers.gyp
File build/sanitizers/sanitizers.gyp (right):

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers....
build/sanitizers/sanitizers.gyp:1: # Copyright (c) 2012 The Chromium Authors.
All rights reserved.
Nit: 2014 + remove the (c) 
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/pres...

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers....
build/sanitizers/sanitizers.gyp:1: # Copyright (c) 2012 The Chromium Authors.
All rights reserved.
I think you want to make a .gypi instead of .gyp, since the latter takes more
time to process (at least that's what I've heard).

[Alexander Potapenko, 2014-06-25 12:46:33]

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers.gyp
File build/sanitizers/sanitizers.gyp (right):

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers....
build/sanitizers/sanitizers.gyp:1: # Copyright (c) 2012 The Chromium Authors.
All rights reserved.
On 2014/06/25 12:27:12, kjellander wrote:
> Nit: 2014 + remove the (c) 
>
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/pres...

Done.

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers....
build/sanitizers/sanitizers.gyp:1: # Copyright (c) 2012 The Chromium Authors.
All rights reserved.
On 2014/06/25 12:27:12, kjellander wrote:
> I think you want to make a .gypi instead of .gyp, since the latter takes more
> time to process (at least that's what I've heard).
Doesn't seem to work that way (at least easily), GYP reports a KeyError.

This will result in creation of a 'sanitizer_options' target in every gyp file,
which will be hard to manage. E.g. we won't be able to easily disable
sanitizer_options via 'dependencies!' like it's done below.

[kjellander_chromium, 2014-06-25 14:17:08]

lgtm as long as someone with better GYP expertise than me takes a look as well.

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers.gyp
File build/sanitizers/sanitizers.gyp (right):

https://codereview.chromium.org/357623003/diff/1/build/sanitizers/sanitizers....
build/sanitizers/sanitizers.gyp:1: # Copyright (c) 2012 The Chromium Authors.
All rights reserved.
On 2014/06/25 12:46:33, Alexander Potapenko wrote:
> On 2014/06/25 12:27:12, kjellander wrote:
> > I think you want to make a .gypi instead of .gyp, since the latter takes
more
> > time to process (at least that's what I've heard).
> Doesn't seem to work that way (at least easily), GYP reports a KeyError.
> 
> This will result in creation of a 'sanitizer_options' target in every gyp
file,
> which will be hard to manage. E.g. we won't be able to easily disable
> sanitizer_options via 'dependencies!' like it's done below.

OK, just make sure someone with more GYP expertise approves this.

[Alexander Potapenko, 2014-06-25 14:28:04]

Nico, can you please take a look? WDYT about making sanitizers.gyp a .gypi?

[Nico, 2014-06-25 16:06:52]

This defines a target, so it should be a gyp, not a gypi I think. (Like it
currently is.)

(Patch makes sense; I'm idly wondering if there'll be situations where webrtc
needs one version of this file and chromium another, and the webrtc dependency
will end up being a headache.)

[Alexander Potapenko, 2014-06-25 16:14:48]

> (Patch makes sense; I'm idly wondering if there'll be situations where webrtc
> needs one version of this file and chromium another, and the webrtc dependency
> will end up being a headache.)
This is my concern as well, but we can just allow WebRTC folks to introduce
#ifdef(WEBRTC) bits into that file.
These options can also be overridden via env at runtime.

[Alexander Potapenko, 2014-06-30 09:45:56]

The CQ bit was checked by glider@chromium.org

[commit-bot: I haz the power, 2014-06-30 09:46:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/glider@chromium.org/357623003/20001

[commit-bot: I haz the power, 2014-06-30 11:42:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-06-30 11:45:27]

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

[commit-bot: I haz the power, 2014-06-30 11:45:27]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[Alexander Potapenko, 2014-06-30 11:49:06]

Henrik, do you have anything to add regarding Nico's concern?

[kjellander_chromium, 2014-06-30 11:54:18]

On 2014/06/30 11:49:06, Alexander Potapenko wrote:
> Henrik, do you have anything to add regarding Nico's concern?

No, this should be fine for us.
If we need to have a different configuration than Chromium we'll have to
maintain a copy of sanitizer_options.cc, but I think it's unlikely it will be
needed.

[kjellander_chromium, 2014-08-20 15:31:29]

On 2014/06/30 11:54:18, kjellander wrote:
> On 2014/06/30 11:49:06, Alexander Potapenko wrote:
> > Henrik, do you have anything to add regarding Nico's concern?
> 
> No, this should be fine for us.
> If we need to have a different configuration than Chromium we'll have to
> maintain a copy of sanitizer_options.cc, but I think it's unlikely it will be
> needed.

Pinging this to ensure this doesn't fall between the cracks :)

[Nico, 2014-08-20 15:58:09]

On 2014/08/20 15:31:29, kjellander wrote:
> On 2014/06/30 11:54:18, kjellander wrote:
> > On 2014/06/30 11:49:06, Alexander Potapenko wrote:
> > > Henrik, do you have anything to add regarding Nico's concern?
> > 
> > No, this should be fine for us.
> > If we need to have a different configuration than Chromium we'll have to
> > maintain a copy of sanitizer_options.cc, but I think it's unlikely it will
be
> > needed.
> 
> Pinging this to ensure this doesn't fall between the cracks :)

It's useful to say who you're pinging. After clicking through, it looks like
this is a ping for glider (to land the cl)

[Alexander Potapenko, 2014-08-21 12:25:14]

Rebased and re-uploaded.
TBRing noelallen@ for a comment-only change in nacl.gyp.

[Alexander Potapenko, 2014-08-21 14:29:17]

The CQ bit was checked by glider@chromium.org

[commit-bot: I haz the power, 2014-08-21 14:31:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/glider@chromium.org/357623003/40001

[commit-bot: I haz the power, 2014-08-21 18:06:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-21 20:34:52]

Committed patchset #3 (40001) as 291168