-
-
Notifications
You must be signed in to change notification settings - Fork 31.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-30631: Silence MSVC warnings (win32) #1963
Conversation
Could you add a bpo issue number to the title? I think there may be an existing issue for this; otherwise please open one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good, but I have a couple of questions.
@@ -9,6 +9,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution | |||
EndProjectSection | |||
EndProject | |||
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "python", "python.vcxproj", "{B11D750F-CD1F-4A96-85CE-E69A5C5259F9}" | |||
ProjectSection(ProjectDependencies) = postProject | |||
{0E9791DB-593A-465F-98BC-681011311618} = {0E9791DB-593A-465F-98BC-681011311618} | |||
EndProjectSection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a project dependency: python -> _ctypes
. This is to avoid ValidateUcrtbase
failing since it uses _ctypes
and runs before _ctypes
is built.
Added by: Build Dependencies ->Project Dependencies...
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
PCbuild/_socket.vcxproj
Outdated
@@ -60,6 +60,9 @@ | |||
<_ProjectFileVersion>10.0.30319.1</_ProjectFileVersion> | |||
</PropertyGroup> | |||
<ItemDefinitionGroup> | |||
<ClCompile> | |||
<PreprocessorDefinitions>_WINSOCK_DEPRECATED_NO_WARNINGS;%(PreprocessorDefinitions)</PreprocessorDefinitions> | |||
</ClCompile> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only change that I have a bit of a problem with: _socket is an internal module, so we should fix warnings rather than papering over them, if possible. How much effort would it take to avoid the deprecated bits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the list of warnings in "socketmodule.c":
..\Modules\socketmodule.c(1067): warning C4996: 'inet_addr': Use inet_pton() or InetPton() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(1850): note: see declaration of 'inet_addr'
..\Modules\socketmodule.c(4408): warning C4996: 'WSADuplicateSocketA': Use WSADuplicateSocketW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(2872): note: see declaration of 'WSADuplicateSocketA'
..\Modules\socketmodule.c(4648): warning C4996: 'WSASocketA': Use WSASocketW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(3457): note: see declaration of 'WSASocketA'
..\Modules\socketmodule.c(4681): warning C4996: 'WSASocketA': Use WSASocketW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(3457): note: see declaration of 'WSASocketA'
..\Modules\socketmodule.c(5119): warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(2238): note: see declaration of 'gethostbyname'
..\Modules\socketmodule.c(5217): warning C4996: 'gethostbyaddr': Use getnameinfo() or GetNameInfoW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(2216): note: see declaration of 'gethostbyaddr'
..\Modules\socketmodule.c(5347): warning C4996: 'WSADuplicateSocketA': Use WSADuplicateSocketW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(2872): note: see declaration of 'WSADuplicateSocketA'
..\Modules\socketmodule.c(5350): warning C4996: 'WSASocketA': Use WSASocketW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(3457): note: see declaration of 'WSASocketA'
..\Modules\socketmodule.c(5669): warning C4996: 'inet_addr': Use inet_pton() or InetPton() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(1850): note: see declaration of 'inet_addr'
..\Modules\socketmodule.c(5712): warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(1868): note: see declaration of 'inet_ntoa'
..\Modules\socketmodule.c(5786): warning C4996: 'WSAStringToAddressA': Use WSAStringToAddressW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(3623): note: see declaration of 'WSAStringToAddressA'
..\Modules\socketmodule.c(5925): warning C4996: 'WSAAddressToStringA': Use WSAAddressToStringW() instead or define _WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings [C:\projects\cpython\PCbuild\_socket.vcxproj]
C:\Program Files (x86)\Windows Kits\8.1\Include\um\winsock2.h(3556): note: see declaration of 'WSAAddressToStringA'
Calling inet_addr
, inet_ntoa
, gethostbyname
and gethostbyaddr
is probably unavoidable. The rest are due to building with ANSI charset and getting the WinAPI defines set to call the ANSI functions rather than the Unicode ones. It is set to build with ANSI explicitly in the _socket.vcxproj
(<CharacterSet>NotSet</CharacterSet>
). I don't know the reasons for this, but there might be other functions in "socketmodule.c" affected by the charset defines. Changing this will likely also break the ability to pass sockets between Python versions before/after the change on Windows using share/fromshare
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is a tough one to fix, but it eventually needs to be fixed.
I deliberately left the warnings there as motivation - we should not suppress them.
Apart from the socket module warnings, this looks fine to me. I don't want those to be suppressed - I want the implementation changed to avoid those functions. |
Thanks, @segevfiner! |
I silenced the specific warnings we are getting and not all warnings from those external libraries that build with warnings.
Those are hand rolled msbuild files and I'm no msbuild expert. So I hope I did it correctly.