Added NVALGRIND define [on by default]. Can be turned off on valgrind slaves ...

Added NVALGRIND define to Release builds [on by default].
This can be turned off on valgrind slaves by using release_valgrind_build=1

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26241

[Timur Iskhodzhanov, 2009-09-14 10:50:59]

Hi Dan, Dean,

I'd like to add NVALGRIND macro to the default set of CPPFLAGS.
On Valgrind/ThreadSanitizer bots this macro will not be defined by using
"valgrind_build=1".

What do you think?

Thank you,
Timur Iskhodzhanov

[not_the_right_dank, 2009-09-14 14:14:55]

That looks reasonably foolproof, but can you arrange
it so valgrind_build defaults to 1 in debug builds?

On Mon, Sep 14, 2009 at 3:50 AM,  <timurrrr@chromium.org> wrote:
>
> Reviewers: dank, Dean McNamee,
>
> Message:
> Hi Dan, Dean,
>
> I'd like to add NVALGRIND macro to the default set of CPPFLAGS.
> On Valgrind/ThreadSanitizer bots this macro will not be defined by using
> "valgrind_build=3D1".
>
> What do you think?
>
> Thank you,
> Timur Iskhodzhanov
>
> Description:
> Added NVALGRIND define [on by default]. Can be turned off on valgrind sla=
ves
> by
> using valgrind_build=3D1
>
> Please review this at http://codereview.chromium.org/195078
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/
>
> Affected files:
> =A0M =A0 =A0 src/build/common.gypi
> =A0M =A0 =A0 tools/buildbot/master.chromium.fyi/master.cfg
> =A0M =A0 =A0 tools/buildbot/master.chromium/master.cfg
>
>
> Index: tools/buildbot/master.chromium/master.cfg
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- tools/buildbot/master.chromium/master.cfg =A0 (revision 26099)
> +++ tools/buildbot/master.chromium/master.cfg =A0 (working copy)
> @@ -571,7 +571,9 @@
> =A0f_sub_dbg_linux =3D m_sub_linux.SubmoduleFactory('sub-dbg-linux',
> target=3D'Debug')
>
> =A0# On linux valgrind bots, override a few build settings.
> -linux_valgrind_gyp_defines =3D 'release_optimize=3D1 werror=3D
> release_extra_cflags=3D"-g"'
> +linux_valgrind_gyp_defines =3D ('valgrind_build=3D1 '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'release_opt=
imize=3D1 werror=3D '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'release_ext=
ra_cflags=3D"-g"')
>
> =A0f_chromium_rel_linux_valgrind_ui =3D F_LINUX(
> =A0 =A0 'chromium-rel-linux-valgrind-ui', target=3D'Release',
> @@ -751,7 +753,8 @@
>
> =A0# On Mac valgrind bots, override the optimizer settings so we don't in=
line
> too
> =A0# much and make the stacks harder to figure out.
> -mac_valgrind_gyp_defines =3D 'mac_release_optimization=3D1'
> +mac_valgrind_gyp_defines =3D ('valgrind_build=3D1 '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'mac_release_opt=
imization=3D1')
>
> =A0# TODO(tvl): it would be nice to better tweak what is built by this to
> speed
> =A0# up the compile cycle times.
> Index: tools/buildbot/master.chromium.fyi/master.cfg
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- tools/buildbot/master.chromium.fyi/master.cfg =A0 =A0 =A0 (revision 2=
6099)
> +++ tools/buildbot/master.chromium.fyi/master.cfg =A0 =A0 =A0 (working co=
py)
> @@ -306,7 +306,9 @@
>
> =A0# On Mac valgrind bots, override the optimizer settings so we don't in=
line
> too
> =A0# much and make the stacks harder to figure out.
> -mac_valgrind_gyp_defines =3D 'mac_release_optimization=3D1
> mac_debug_optimization=3D1'
> +mac_valgrind_gyp_defines =3D ('valgrind_build=3D1 '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'mac_release_opt=
imization=3D1 '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'mac_debug_optim=
ization=3D1')
>
> =A0# temporary hack: use debug builds for mac valgrind ui tests
> =A0# until we figure out why we can't get stacks on release builds
> @@ -338,7 +340,8 @@
> =A0 =A0 =A0 =A0 =A0 =A0 =A0'-target', 'ui_tests'])
>
> =A0# On linux valgrind bots, override a few build settings.
> -linux_valgrind_gyp_defines =3D ('release_optimize=3D1 werror=3D '
> +linux_valgrind_gyp_defines =3D ('valgrind_build=3D1 '
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0'release_opt=
imize=3D1 werror=3D '
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 'release_extr=
a_cflags=3D"-g"')
>
> =A0f_sub_rel_linux_tsan =3D m_linux.ChromiumFactory('sub-rel-linux-tsan',
> Index: src/build/common.gypi
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- src/build/common.gypi =A0 =A0 =A0 (revision 26099)
> +++ src/build/common.gypi =A0 =A0 =A0 (working copy)
> @@ -104,6 +104,8 @@
>
> =A0 =A0 'chromeos%': 0,
>
> + =A0 =A0'valgrind_build%': 0,
> +
> =A0 =A0 # The system root for cross-compiles. Default: none.
> =A0 =A0 'sysroot%': '',
>
> @@ -189,6 +191,9 @@
> =A0 =A0 =A0 ['chromeos=3D=3D1', {
> =A0 =A0 =A0 =A0 'defines': ['OS_CHROMEOS=3D1'],
> =A0 =A0 =A0 }],
> + =A0 =A0 =A0['valgrind_build=3D=3D0', {
> + =A0 =A0 =A0 =A0'defines': ['NVALGRIND'],
> + =A0 =A0 =A0}],
> =A0 =A0 =A0 ['coverage!=3D0', {
> =A0 =A0 =A0 =A0 'conditions': [
> =A0 =A0 =A0 =A0 =A0 ['OS=3D=3D"mac"', {
>
>
>

[Timur Iskhodzhanov, 2009-09-14 19:31:59]

On 2009/09/14 14:14:55, not_the_right_dank wrote:
> That looks reasonably foolproof, but can you arrange
> it so valgrind_build defaults to 1 in debug builds?
Updated the patch set.
Did I get your suggestion correctly?

[dank, 2009-09-15 18:20:44]

LGTM 
better announce this afterwards, though.  (e.g. "By default, release builds will
no longer be valgrindable.  Here's how to turn on valgrind support if you need
it.")  Maybe put that in the chromium wiki valgrind page

[Timur Iskhodzhanov, 2009-09-15 18:25:40]

Hi Nicolas,

Can you please review this changelist?

Thank you,
Timur Iskhodzhanov

[Nicolas Sylvain, 2009-09-15 18:39:30]

LGTM

[Timur Iskhodzhanov, 2009-09-15 18:42:05]

On 2009/09/15 18:39:30, Nicolas Sylvain wrote:
> LGTM
Thank you!
Submitted