[vlc-devel] [PATCH] configure: Check the toolchain default _WIN32_WINNT in addition to a command line override

[Martin Storsjö]

On Fri, 3 Apr 2020, Martin Storsjö wrote:

> On Fri, 3 Apr 2020, Steve Lhomme wrote:
>
>> On 2020-04-03 13:57, Martin Storsjö wrote:
>>> On Fri, 3 Apr 2020, Steve Lhomme wrote:
>>> 
>>>> On 2020-04-03 13:27, Martin Storsjö wrote:
>>>>> Since 255e2ce27, we try not to override _WIN32_WINNT in case it already
>>>>> is defined on the command line to a higher value. However, if it isn't
>>>>> specified on the command line, but the toolchain headers default to
>>>>> a newer version, we should also honor it and keep that version instead
>>>>> of forcing a lower version here. (If the toolchain defaults to a newer
>>>>> version, runtime libs of the toolchain may rely on such a new version
>>>>> anyway, so forcing a lower target within VLC might be useless.)
>>>> 
>>>> This seems OK although it may create issues with code like this:
>>>> #ifdef HAVE_CONFIG_H
>>>> # include "config.h"
>>>> #endif
>>>> 
>>>> #if !defined(_WIN32_WINNT) || _WIN32_WINNT < 0x0601 // _WIN32_WINNT_WIN7
>>>> # undef _WIN32_WINNT
>>>> # define _WIN32_WINNT 0x0601 // _WIN32_WINNT_WIN7
>>>> #endif
>>> 
>>>> 
>>>> In this case the user may not set value in the command-line and the 
>>>> default one from the toolchain is used. But it may be higher than Win7 
>>>> but then we force it down.
>>> 
>>> Hmm, you're right...
>>> 
>>> To handle that case gracefully, we would either need to include _mingw.h 
>>> (which sets such defaults, if __MINGW32__ is defined) between config.h and 
>>> the _WIN32_WINNT adjustment, or try to parse out the toolchain default 
>>> value in configure and set that in config.h, instead of the VLC default.
>> 
>> That would be nice but it sounds tricky.
>> Or maybe using the output of such a program in the configure script:
>> 
>> #include <windows.h>
>> #include <stdio.h>
>> int main(void) {
>>   printf("0x%04X", _WIN32_WINNT);
>>   return 0;
>> }
>
> Yeah, except we can't count on executing test apps - so it'd have to be 
> something like "$CC -E | tail -1" on something like this:
>
> #include <windows.h>
> _WIN32_WINNT
>
>
>>>> On the other hand it's not a regression. In that case we use to set the 
>>>> value in config.h to Win7 and it would never pick the default toolchain 
>>>> one. But we need to keep this in mind. Especially when backporting to 3.0 
>>>> (because win7 is a bad example since that's the minimum in 4.0 anyway).
>>> 
>>> Well this is only an issue for the specific source files where we override 
>>> the common version, right?
>>> 
>>>> I'm OK with the change if we decide we don't want people to compile 4.0 
>>>> for anything lower than Win7.
>>> 
>>> I don't see how this patch is an issue for that particular case though. If 
>>> you intentionally want to compile for a lower version, pass 
>>> -D_WIN32_WINNT=0x0600 in CFLAGS/CXXFLAGS, then it will always be defined
>> 
>> Ah yes, so it's only for toolchain that would have a default (but not force 
>> via the command-line) version higher than Win7 where we might benefit of 
>> new API. I don't know if that's possible in mingw64, like when 
>> WINSTORECOMPAT is set. I don't think that's the case with MSVC.
>
> Yes, it's possible - mingw-w64 normally default to 0x502, but it's settable 
> when installing mingw-w64-headers (--with-default-win32-winnt=whatever) - 
> llvm-mingw defaults to 0x0601.
>
> So therefore I think one gets a more consistent experience if config.h 
> doesn't force it to a lower version. On 4.0/master it's not an issue as it 
> sets the same version as the toolchain, but the generic mechanism would be 
> good for 3.0 I think, to avoid forcing 0x502 on llvm-mingw there.

So if there's no objections to this one, I'd push this one for master, and 
then send new suggested backports for 3.0.

// Martin