Den lör 25 apr. 2026 kl 18:04 skrev Branko Čibej <[email protected]>: > > On 25. 4. 26 17:29, Daniel Sahlberg wrote: > > Den lör 25 apr. 2026 kl 14:09 skrev Daniel Sahlberg > > <[email protected]>: > >> Den ons 22 apr. 2026 kl 15:35 skrev Ivan Zhakov<[email protected]>: > >>> On Mon, 6 Apr 2026 at 18:45, Daniel Sahlberg<[email protected]> > >>> wrote: > >>> > >>>> Den tors 19 mars 2026 kl 21:37 skrev Timofei Zhakov<[email protected]>: > >>>> > >>>>> On Wed, Mar 18, 2026 at 1:27 AM Branko Čibej<[email protected]> wrote: > >>>>>> On 17. 3. 26 21:35, Timofei Zhakov wrote: > >>>>>>>>>> Just add 14.5 to the list. And probably remove anything older > >>>> than, > >>>>>>>>>> what, 10? I don't think the versions of Windows that used those > >>>>>>>>>> compilers even exist any more, except possibly in some strange > >>>>> airgapped > >>>>>>>>>> environment that may as well use an older version of Serf. > >>>>>>>>> I'm concerned that by just adding it to the list, because as a new > >>>>>>>>> version (of MSVC) comes out, it will break the build once again. > >>>> This > >>>>>>>>> at least "patches" the problem so I'm still planning to prepare a > >>>> fix > >>>>>>>>> for that. > >>>>>>>>> > >>>>>>>>> I believe there should be a more elegant and generalised solution > >>>> for > >>>>>>>>> the issue of detecting the compiler. > >>>>>>>> We're not detecting the compiler. This appears to be a cross-check > >>>>>>>> between MSVC version and SCons support for it. See SConstruct around > >>>>>>>> line 187 `if sys.platform == 'win32':`. > >>>>>>>> > >>>>>>>> The last changes in this area were in r1909315 and r1909316. I think > >>>>> we > >>>>>>>> can handle one commit every 2 years to track this. We can look for a > >>>>>>>> better way once we decide which build system to keep, CMake or > >>>> SCons. > >>>>>>> Okay, can serf release a patch to the build system in a few weeks > >>>>>>> after a new version of MSVC is out? I honestly don't think so. > >>>>>>> > >>>>>>>> As the saying goes, premature optimisation is the root of all evil. > >>>>>>> That is true, but this is not an optimisation but a design problem. > >>>>>>> > >>>>>>> By the way, I found a related issue on scons' github with ~100 > >>>>>>> comments [1]. They don't seem to come up with a canonical solution. > >>>>>>> However, I could have missed something from it. > >>>>>>> > >>>>>>> [1]https://github.com/SCons/scons/issues/3664 > >>>>>> That's an "interesting" read. > >>>>>> > >>>>>> Maybe try removing the MSVC_VERSION completely? But that would break > >>>>>> older SCons versions... > >>>>> Can we perhaps remove the mapping for the existing versions of Visual > >>>>> Studio per its MSVC version? > >>>>> > >>>>> This would revert r1712131 which was justified as: > >>>>> > >>>>> [[[ > >>>>> * SConstruct > >>>>> Let scons generate the valid options. > >>>>> Add the most likely next version of Visual Studio to the list. > >>>>> ]]] > >>>>> > >>>>> -- > >>>>> Timofei Zhakov > >>>>> > >>>> A bit late to the party, sorry about that! > >>>> > >>>> From what I understand, this was initially added in r1699590: "Allow > >>>> specifying which Visual C++ compiler to use when multiple versions are > >>>> installed.". Already at that time there was a specific > >>>> allowed_values-list. > >>>> SCons seems to support a specific list of MSVC versions (see [1]). The > >>>> map > >>>> is only to allow you to say MSVC_VERSION=2022 and have SCons change to > >>>> the > >>>> internal value of 14.3. > >>>> > >>>> I don't see why we should restrict Serf to a specific MSVC version as > >>>> long > >>>> as SCons support it. Thus I suggest that we remove the allowed_values > >>>> argument. Anyone disagree? > >>>> > >>> I am +1 to remove MSVC version mapping. > >>> > >> I have reviewed this once more and I no longer believe removing the > >> map is the correct solution. > >> > >> Timofei's initial problem was that he was using MSVC_VERSION=14.5. > >> That fails because 14.5 is not in the allow_list. > >> > >> If he tried using MSVC_VERSION=2026, /that/ would fail because 2006 is > >> missing in the map. > >> > >> To solve Timofei's initial problem, we have to: > >> * Add 14.5 in the allow_list > >> * Or remove the allow_list > >> > >> If we do the first we could as well add the mapping '2026' : '14.5'. > >> > >> To do the latter, we have to change MSVC_VERSION from an EnumVariable > >> to a plain variable - I think something like this should work: > >> > >> [[[ > >> Index: SConstruct > >> =================================================================== > >> --- SConstruct (revision 1933326) > >> +++ SConstruct (working copy) > >> @@ -203,21 +203,7 @@ > >> 'ARM64': 'arm64' > >> }), > >> > >> - EnumVariable('MSVC_VERSION', > >> - "Visual C++ to use for building", > >> - None, > >> - allowed_values=('14.3', '14.2', '14.1', '14.0', '12.0', > >> - '11.0', '10.0', '9.0', '8.0', '6.0'), > >> - map={'2005' : '8.0', > >> - '2008' : '9.0', > >> - '2010' : '10.0', > >> - '2012' : '11.0', > >> - '2013' : '12.0', > >> - '2015' : '14.0', > >> - '2017' : '14.1', > >> - '2019' : '14.2', > >> - '2022' : '14.3', > >> - }), > >> + ('MSVC_VERSION', "Visual C++ to use for building"), > >> > >> # We always documented that we handle an install layout, but in fact > >> we > >> # hardcoded source layouts. Allow disabling this behavior. > >> ]]] > >> > >> That of course removes all validation - someone have to check what > >> happens if you specify an invalid version. > > That someone turned out to be me. This is what happens when I apply > > the above patch and ask for some "stupid" version: > > > > [[[ > > C:\Devel\serf_trunk>scons MSVC_VERSION=14.12 > > scons: Reading SConscript files ... > > warning: replaced Conftest.CheckFunc() for SCons version < 4.7. > > > > scons: warning: MSVC version '14.12' was not found. > > Visual Studio C/C++ compilers may not be set correctly. > > Installed versions are: ['14.3'] > > File "C:\Devel\serf_trunk\SConstruct", line 216, in <module> > > > > [... and then a little later ...] > > > > scons: done reading SConscript files. > > scons: Building targets ... > > cl /Fosrc\config_store.obj /c src\config_store.c /nologo /W4 /wd4100 > > /wd4127 /wd4706 /we4013 /O2 /MD /DNDEBUG /DOPENSSL_NO_DEPRECATED > > /DOPENSSL_NO_STDIO /DWIN32 /DWIN32_LEAN_AND_MEAN /DNOUSER /DNOGDI > > /DNONLS /DNOCRYPT /D_CRT_SECURE_NO_WARNINGS /D_CRT_NONSTDC_NO_WARNINGS > > /DSERF_NO_SSL_BIO_WRAPPERS /DSERF_NO_SSL_X509_STORE_WRAPPERS > > /DSERF_NO_SSL_X509_GET0_NOTBEFORE /DSERF_NO_SSL_X509_GET0_NOTAFTER > > /DSERF_NO_SSL_X509_GET0_CHAIN /DSERF_NO_SSL_ASN1_STRING_GET0_DATA > > /DSERF_HAVE_SSPI /I. /Iinstall\include /Iinstall\include /Iinstall > > /Iinstall\inc32 /Z7 > > 'cl' is not recognized as an internal or external command, > > operable program or batch file. > > scons: *** [src\config_store.obj] Error 1 > > scons: building terminated because of errors. > > ]]] > > > > Note how it lists the installed versions (I'm on Visual Studio 2022 > > which corresponds to 14.3, so all good). > > > > I think it makes sense to remove the explicit list. I committed this > > as r1933327. > > Did you by any chance test this with Scons-2.3.5, which is the earliest > version we currently support? We went out of our way to keep supporting > that version, but there's no reason we can't upgrade to something less > antediluvian on trunk and hence 1.5. > > -- Brane
Hm.. no I only tested on 4.6. Documentation (see [1]) indicate that behaviour should be the same: not setting it should default to the latest version. Current behaviour might even be better, since setting it to 14.3 (which was accepted by the allow_list) wouldn't work with SCons 2.3.5, failing just as in my example with MSVC_VERSION=14.12 above. I'll try to figure out a way to run SCons 2.3.5 and test. Thanks for reminding me of this! Cheers, Daniel [1] https://scons.org/doc/2.3.5/HTML/scons-user.html#app-variables
