coturn
Explicitly state minimum Windows version
This pull request is a split of PR #1061
Depending on build system the code will be built against different Windows versions (see comments in old PR).
According to https://learn.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt you should define _WIN32_WINNT to the oldest supported platform. And since https://github.com/coturn/coturn/issues/1044 states coturn should support >= Windows 7, I have set it to 0x0601.
The macro is defined by sdk. https://learn.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers#setting-winver-or-_win32_winnt understand the note part
Only you are sure that your current sdk version greater your need sdk version. you are defined it. or you are sure that your current sdk version greater 0x7001, then you are define it, support xp etc. But we can't assume it. So that we are not define it.
You can make this function by CMAKE. See: https://cmake.org/cmake/help/latest/variable/CMAKE_GENERATOR_TOOLSET.html#variable:CMAKE_GENERATOR_TOOLSET https://cmake.org/cmake/help/latest/variable/CMAKE_GENERATOR_PLATFORM.html#variable:CMAKE_GENERATOR_PLATFORM
The macro is defined by sdk. https://learn.microsoft.com/en-us/windows/win32/WinProg/using-the-windows-headers#setting-winver-or-_win32_winnt understand the note part
The note part is exactly why I want to introduce this! As we concluded in the old PR different, different SDKs/runtimes set different min versions. MSVC2017 defaults to 0x0a00, your Msys minGW defaults to 0x0601, and my mingw64 cross compile environment defaults to 0x0501.
By explicitly state the min version we gain 2 things:
- Avoid compile errors on a runtime that defaults to a lower min version than coturn requires.
- Avoid that a future contributor accidentally introduce code that only works on recent Windows-versions, while coturn still want to support older windows versions.
It is not like coturn would be the only project in the world that does this. Examples: MySQL: https://github.com/mysql/mysql-server/blob/8.0/cmake/os/Windows.cmake cURL (although a bit more complicated): https://github.com/curl/curl/blob/master/CMakeLists.txt
Ahh, interesting! To be honest I don't fully understand how they should be used. Can you post an example?
The "cmake -T" solution would only work with visual studio environments, not mingw right? At least it doesn't work in a cross compile environment with mingw.
And if it would work with mingw on Windows, and someone decide cross compile should not be supported, it should still be stated in a cmake-file to achive the points in my comments above.
Macros starting with "_" are defined and used internally by the compiler. It is not recommended to use it directly in the program.It is recommended to close.
The "cmake -T" solution would only work with visual studio environments, not mingw right? At least it doesn't work in a cross compile environment with mingw.
And if it would work with mingw on Windows, and someone decide cross compile should not be supported, it should still be stated in a cmake-file to achive the points in my comments above.
@pando-emil This program does not require it. If you are sure you need to, you can pass this value via the CMake parameter.
ag:
cmake -D_WIN32_WINNT=
Macros starting with "_" are defined and used internally by the compiler. It is not recommended to use it directly in the program.It is recommended to close.
This is wrong, Microsoft explicitly demonstrates and states you SHOULD define them yourself, as demonstrated here, just like @pando-emil stated in the top of this PR.
https://learn.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt
"To modify the macros, in a header file (for example, in targetver.h, which is included by some project templates that target Windows), add the following lines.
C
#define WINVER 0x0A00
#define _WIN32_WINNT 0x0A00
The macros in the example are set to target every version of the Windows 10 operating system. The possible values are listed in the Windows header file sdkddkver.h, which defines macros for each major Windows version. To build your application to support a previous Windows platform, include WinSDKVer.h. Then, set the WINVER and _WIN32_WINNT macros to the oldest supported platform before including sdkddkver.h. Here are the lines from the Windows 10 SDK version of sdkddkver.h that encode the values for each major version of Windows:"
@Neustradamus Sorry for the long absense!
I still have no better solution than I proposed with the patch. I simply don't agree with KangLin that there is anything wrong with doing as proposed, but if there is no interest in including the patch, I'll just close the PR.
As I tried to explain above, the patch aims at making the build system more robust to different build environments and future contributions. It is not an issue for me personally, I already figured out what I need to fix outside the build system. :)