Make all jingle targets build on Win64

Make all jingle targets build on Win64

BUG=166496
BUG=167187
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176355

[jschuh, 2013-01-10 02:17:58]

Just error suppressions that we're already ignoring on other 64-bit builds.
We'll have a cleanup cycle later after we get a win64 bot stood up.

[awong, 2013-01-10 02:26:49]

I have to run of the day, but sergeyu@ would be better for this change
since he's watching the jingle code now.


On Wed, Jan 9, 2013 at 6:17 PM, <jschuh@chromium.org> wrote:

> Reviewers: awong,
>
> Message:
> Just error suppressions that we're already ignoring on other 64-bit builds.
> We'll have a cleanup cycle later after we get a win64 bot stood up.
>
> Description:
> Make all jingle targets build on Win64
>
> BUG=166496
> BUG=167187
>
> Please review this at
https://codereview.chromium.**org/11818046/<https://codereview.chromium.org/1...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>   M     jingle/jingle.gyp
>   M     third_party/libjingle/**libjingle.gyp
>
>
> Index: jingle/jingle.gyp
> ==============================**==============================**=======
> --- jingle/jingle.gyp   (revision 175573)
> +++ jingle/jingle.gyp   (working copy)
> @@ -45,6 +45,8 @@
>              '../third_party/libjingle/**libjingle.gyp:libjingle',
>              '../third_party/libjingle/**libjingle.gyp:libjingle_p2p',
>            ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [4267, ],
>          },
>          # A library for sending and receiving peer-issued notifications.
>          #
> @@ -104,8 +106,14 @@
>            ],
>            'defines' : [
>              '_CRT_SECURE_NO_WARNINGS',
> -            '_USE_32BIT_TIME_T',
>            ],
> +          'target_conditions': [
> +            ['target_arch == "ia32"', {
> +              'defines': [
> +                '_USE_32BIT_TIME_T',
> +              ],
> +            }],
> +          ],
>            'dependencies': [
>              '../base/base.gyp:base',
>              '../build/temp_gyp/googleurl.**gyp:googleurl',
> @@ -123,6 +131,8 @@
>                ],
>              }],
>            ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [4267, ],
>          },
>          {
>            'target_name': 'notifier_test_util',
> @@ -154,6 +164,8 @@
>              'jingle_glue',
>              '../base/base.gyp:base',
>            ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [4267, ],
>          },
>          {
>            'target_name': 'jingle_unittests',
> @@ -212,6 +224,8 @@
>              '../testing/gtest.gyp:gtest',
>              '../third_party/libjingle/**libjingle.gyp:libjingle',
>            ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [4267, ],
>          },
>        ],
>      }, {  # enable_webrtc != 1 and (OS == "ios" or OS == "android")
> Index: third_party/libjingle/**libjingle.gyp
> ==============================**==============================**=======
> --- third_party/libjingle/**libjingle.gyp (revision 175573)
> +++ third_party/libjingle/**libjingle.gyp (working copy)
> @@ -24,11 +24,17 @@
>        'NO_MAIN_THREAD_WRAPPING',
>        'NO_SOUND_SYSTEM',
>        'SRTP_RELATIVE_PATH',
> -      '_USE_32BIT_TIME_T',
>        # TODO(ronghuawu): Remove this once libjingle is updated to use the
> new
>        # webrtc.
>        'USE_WEBRTC_DEV_BRANCH',
>      ],
> +    'target_conditions': [
> +      ['target_arch == "ia32"', {
> +        'defines': [
> +          '_USE_32BIT_TIME_T',
> +        ],
> +      }],
> +    ],
>      'configurations': {
>        'Debug': {
>          'defines': [
> @@ -419,7 +425,8 @@
>              '<(libjingle_source)/talk/**base/winping.h',
>            ],
>            # Suppress warnings about WIN32_LEAN_AND_MEAN.
> -          'msvs_disabled_warnings': [ 4005 ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [ 4005, 4267 ],
>          }],
>          ['os_posix == 1', {
>            'sources': [
> @@ -550,6 +557,8 @@
>        'dependencies': [
>          'libjingle',
>        ],
> +      # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +      'msvs_disabled_warnings': [4267, ],
>      },  # target libjingle_p2p
>      {
>        'target_name': 'peerconnection_server',
> @@ -569,6 +578,8 @@
>        'dependencies': [
>          'libjingle',
>        ],
> +      # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +      'msvs_disabled_warnings': [4267, 4309, ],
>      }, # target peerconnection_server
>    ],
>    'conditions': [
> @@ -788,6 +799,8 @@
>              'libjingle',
>              'libjingle_p2p',
>            ],
> +          # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
> +          'msvs_disabled_warnings': [4267, ],
>          },  # target libjingle_peerconnection
>        ],
>      }],
>
>
>

[Sergey Ulanov, 2013-01-10 02:37:39]

I have couple of questions.

https://codereview.chromium.org/11818046/diff/5003/jingle/jingle.gyp
File jingle/jingle.gyp (right):

https://codereview.chromium.org/11818046/diff/5003/jingle/jingle.gyp#newcode114
jingle/jingle.gyp:114: '_USE_32BIT_TIME_T',
time_t is not used in the code compiled by this target. Is it possible to move
this define and msvs_disabled_warnings to direct_dependent_settings section in
libjingle.gyp?

https://codereview.chromium.org/11818046/diff/5003/third_party/libjingle/libj...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/11818046/diff/5003/third_party/libjingle/libj...
third_party/libjingle/libjingle.gyp:13: 'target_arch%': "ia32",
Why is this necessary? shouldn't we expect that target_arch is always defined?

[jschuh, 2013-01-10 03:12:15]

https://codereview.chromium.org/11818046/diff/5003/jingle/jingle.gyp
File jingle/jingle.gyp (right):

https://codereview.chromium.org/11818046/diff/5003/jingle/jingle.gyp#newcode114
jingle/jingle.gyp:114: '_USE_32BIT_TIME_T',
On 2013/01/10 02:37:39, sergeyu wrote:
> time_t is not used in the code compiled by this target. Is it possible to move
> this define and msvs_disabled_warnings to direct_dependent_settings section in
> libjingle.gyp?

Given it's current location I assumed it was used at this level, but I'm happy
to move it.

https://codereview.chromium.org/11818046/diff/5003/third_party/libjingle/libj...
File third_party/libjingle/libjingle.gyp (right):

https://codereview.chromium.org/11818046/diff/5003/third_party/libjingle/libj...
third_party/libjingle/libjingle.gyp:13: 'target_arch%': "ia32",
On 2013/01/10 02:37:39, sergeyu wrote:
> Why is this necessary? shouldn't we expect that target_arch is always defined?

That's what I thought, but according to the errors I'm getting on the bots
that's not the case.

[jschuh, 2013-01-10 03:46:32]

Is this along the lines of what you were thinking?

[jschuh, 2013-01-10 21:01:14]

@sergeyu - Turns out I just needed to do it as a nested condition to get the
evaluation to occur at the right time. So, no defaults are needed. Are there any
further changes you want or are we good to go?

[Sergey Ulanov, 2013-01-10 21:29:04]

LGTM!

[commit-bot: I haz the power, 2013-01-11 03:05:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/11818046/20003

[commit-bot: I haz the power, 2013-01-11 13:34:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/11818046/20003

[commit-bot: I haz the power, 2013-01-11 13:43:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/11818046/20003

[commit-bot: I haz the power, 2013-01-11 13:52:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jschuh@chromium.org/11818046/20003

[commit-bot: I haz the power, 2013-01-11 15:59:47]

Change committed as 176355