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. Cheers, Daniel
