Add skia_clang_build gyp define, add -Wstring-conversion flag

Add skia_clang_build gyp define, add -Wstring-conversion flag

If you want the warning, you have to specify skia_clang_build=1

R=djsollen@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10861

[borenet, 2013-08-21 14:00:58]

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:285: [ 'skia_clang_build', {
It's kind of unfortunate that we have to place this flag behind a variable which
must be explicitly supplied, but I don't know of a better way.  I guess we could
shell out to see if "clang" is in "echo $CC", but that seems fragile.

[djsollen, 2013-08-21 14:09:37]

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:287: '-Wstring-conversion',
This will break our bots if we check it in.  Do we have a plan to handle that?

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi#new...
gyp/common_variables.gypi:135: 'skia_asan_build%': '<(skia_asan_build)',
don't you need to define skia_clang_build here as well?

[robertphillips, 2013-08-21 14:12:51]

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:287: '-Wstring-conversion',
Do you mean because all the !""'s outside of what Chromium normally sees will
become errors?

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi#new...
gyp/common_variables.gypi:123: }, {
Should this % be here and not above?

[borenet, 2013-08-21 14:16:01]

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi
File gyp/common_conditions.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_conditions.gypi#ne...
gyp/common_conditions.gypi:287: '-Wstring-conversion',
On 2013/08/21 14:12:51, robertphillips wrote:
> Do you mean because all the !""'s outside of what Chromium normally sees will
> become errors?

This doesn't change the behavior of our bots, since they don't yet provide the
"skia_clang_build" define.  I'll have to add that in a separate CL, which will
then cause the bots to fail if the warning hasn't been fixed.

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi
File gyp/common_variables.gypi (right):

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi#new...
gyp/common_variables.gypi:123: }, {
On 2013/08/21 14:12:51, robertphillips wrote:
> Should this % be here and not above?

Building with ASAN implies that we're using Clang, so we force
skia_clang_build=1 in that case.  Otherwise, we use what's provided in
GYP_DEFINES with 0 as the default.

https://codereview.chromium.org/23038014/diff/1/gyp/common_variables.gypi#new...
gyp/common_variables.gypi:135: 'skia_asan_build%': '<(skia_asan_build)',
On 2013/08/21 14:09:37, djsollen wrote:
> don't you need to define skia_clang_build here as well?

No, since skia_clang_build is already defined at this level.

[borenet, 2013-08-21 17:24:02]

ping

[djsollen, 2013-08-21 17:25:45]

lgtm

[borenet, 2013-08-21 18:02:54]

Committed patchset #1 manually as r10861 (presubmit successful).