WIP: Disable warning C4996 for Win SDK 8.1 for all

Disable warning C4996 for Win SDK 8.1

This patch disables warning C4996 for MSVS2013, which is caused by using deprecated Windows API GetVersion() and GetVersionEx().

BUG=311488
TEST=n/a; no behavior changes

[Will Harris, 2013-10-18 11:37:57]

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

https://codereview.chromium.org/28823003/diff/1/build/common.gypi#newcode4394
build/common.gypi:4394: 'msvs_disabled_warnings': [4996]
this turns off warnings for all deprecated functions
perhaps we should wrap calls to GetVersionEx in a pragma
warning disable rather than do this globally?

[yosin_UTC9, 2013-10-18 12:44:05]

On 2013/10/18 11:37:57, Will Harris wrote:
> https://codereview.chromium.org/28823003/diff/1/build/common.gypi
> File build/common.gypi (right):
> 
> https://codereview.chromium.org/28823003/diff/1/build/common.gypi#newcode4394
> build/common.gypi:4394: 'msvs_disabled_warnings': [4996]
> this turns off warnings for all deprecated functions
> perhaps we should wrap calls to GetVersionEx in a pragma
> warning disable rather than do this globally?

There are 20+ GetVersionEx() usage most of them in third_party/.
 
https://code.google.com/p/chromium/codesearch#search/&q=GetVersionEx%5C(%20fi...

It is better to change third_party/*.gyp and wrapping GetVersionEx() for
chromium code?

[scottmg, 2013-10-18 16:09:00]

I didn't encounter these warnings when building on VS2013,
http://crbug.com/288948. Can you paste the warnings you're seeing or maybe any
unusual build settings?

[scottmg, 2013-10-18 16:11:56]

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

https://codereview.chromium.org/28823003/diff/1/build/common.gypi#newcode4392
build/common.gypi:4392: ['MSVS_VERSION=="2013"', {
needs 'or MSVS_VERSION=="2013e"' also. If it's mostly in third_party, it might
be better to put it in the chromium_code==0 block if possible.

[yosin_UTC9, 2013-10-21 01:35:24]

On 2013/10/18 16:09:00, scottmg wrote:
> I didn't encounter these warnings when building on VS2013,
> http://crbug.com/288948. Can you paste the warnings you're seeing or maybe any
> unusual build settings?

Error message is pasted.
It seems this warning comes from Windows SDK 8.1.

FAILED: ninja -t msvc -e environment.x64 -- "C:\Program Files (x86)\Microsoft
Visual Studio 12.0\VC\bin\amd64\cl.exe" /nologo /showI
ncludes /FC @obj\base\win\base_nacl_win64.windows_version.obj.rsp /c
..\..\base\win\windows_version.cc /Foobj\base\win\base_nacl_win
64.windows_version.obj /Fdobj\base\base_nacl_win64.pdb
d:\src\w\cr\src\base\win\windows_version.cc(37) : error C2220: warning treated
as error - no 'object' file generated
d:\src\w\cr\src\base\win\windows_version.cc(37) : warning C4996:
'GetVersionExW': was declared deprecated c:\program files (x86)\windows
kits\8.1\include\um\sysinfoapi.h(442) : see declaration of 'GetVersionExW'

[cpu_(ooo_6.6-7.5), 2013-10-21 18:01:30]

We haven't decided if we want to support the win8.1 sdk, moving from win7 sdk to
win8 sdk was very painful.

Besides this warning what else needs to be fixed to produce a build with the
win8.1 sdk?

[scottmg, 2013-10-21 18:03:27]

(Please update the CL description to say 8.1 SDK rather than VS2013 too)

[yosin_UTC9, 2013-10-22 03:49:57]

On 2013/10/21 18:01:30, cpu wrote:
> We haven't decided if we want to support the win8.1 sdk, moving from win7 sdk
to
> win8 sdk was very painful.
> 
> Besides this warning what else needs to be fixed to produce a build with the
> win8.1 sdk?

Other than disabling C4996, MSVS 2012 can build chrome.

[Will Harris, 2013-10-22 09:13:41]

On 2013/10/22 03:49:57, Yoshifumi Inoue wrote:
> On 2013/10/21 18:01:30, cpu wrote:
> > We haven't decided if we want to support the win8.1 sdk, moving from win7
sdk
> to
> > win8 sdk was very painful.
> > 
> > Besides this warning what else needs to be fixed to produce a build with the
> > win8.1 sdk?
> 
> Other than disabling C4996, MSVS 2012 can build chrome.

I've been building Chrome fine for a few months now with MSVS 2012
and win8 sdk - and I haven't had to disable C4996.  Do we need to
move to win8.1 SDK?

[cpu_(ooo_6.6-7.5), 2013-10-24 00:29:51]

Ok lets do this. lgtm

This does not mean we are endorsing the use of win8.1 sdk though. It is still
use at your own risk.

[yosin_UTC9, 2013-10-24 01:44:16]

Is there way to check Win SDK version in gyp file? MSVS=2013 doesn't imply Win
SDK 8.1.

If there is no way to check Win SDK version in gyp file, I should cancel this
patch.

[scottmg, 2013-10-24 02:19:29]

On 2013/10/24 01:44:16, Yoshifumi Inoue wrote:
> Is there way to check Win SDK version in gyp file? MSVS=2013 doesn't imply Win
> SDK 8.1.

I don't know of a way to do that directly. If there's something a python script
could do to check, then it could set a gyp variable using a form similar to
dir_exists.py
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...
and then the warning disable could be based on that variable.
 
> If there is no way to check Win SDK version in gyp file, I should cancel this
> patch.

You might be also be able to do something with a ForcedIncludeFiles block that
checks a #define and then does a #pragma, but since it's in third_party, that
will probably still require changing dependencies so might not be too useful.

[yosin_UTC9, 2013-10-24 04:21:04]

scottmg@, thanks for information.

I'll write has_str.py then

['OS=="win" and chromium_code==0 and "<!(python <(DEPTH)/build/has_str.py
<_WIN32_WINNT_WINBLUE (windows_sdk_path)/sdkddkver.h)"=="True"', {
  'msvs_disabled_warnings': [4996]
}],

Before doing so, I'll make Chromium code to free from GetVersionEx.

I close this patch and make other patches to do so.

[scottmg, 2013-10-24 04:26:33]

On 2013/10/24 04:21:04, Yoshifumi Inoue wrote:
> scottmg@, thanks for information.
> 
> I'll write has_str.py then
> 
> ['OS=="win" and chromium_code==0 and "<!(python <(DEPTH)/build/has_str.py
> <_WIN32_WINNT_WINBLUE (windows_sdk_path)/sdkddkver.h)"=="True"', {
>   'msvs_disabled_warnings': [4996]
> }],
> 
> Before doing so, I'll make Chromium code to free from GetVersionEx.
> 
> I close this patch and make other patches to do so.

sgtm. Now that you mention it, since it's Windows only, you might be able to get
away with just using "findstr" instead of a separate python script if you're a
bit careful to make sure it only runs on Windows.

[adibu2008_gmail.com, 2014-03-31 23:11:31]

On Sunday, October 20, 2013 9:35:37 PM UTC-4, yo...@chromium.org wrote:
>
> https://codereview.chromium.org/28823003/ 
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.