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

[Hugo Beauzée-Luyssen]

Fine by me, I'd rather not rely on _mingw.h
However, I'd feel more at ease if this gets backported after we tag :)

On Mon, Apr 6, 2020, at 2:04 PM, Jean-Baptiste Kempf wrote:
> OK for me.
> 
> On Mon, Apr 6, 2020, at 14:01, Steve Lhomme wrote:
> > OK for me.
> > 
> > On 2020-04-06 13:51, Martin Storsjö wrote:
> > > 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
> > > 
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > > 
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> 
> -- 
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr