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.

Now I have tested. It "sort of" works, but in the end it fails. When I
ask for MSVC_VERSION=14.3 it correctly pick up my selection, but it
doesn't find that version. This is to be expected since Scons 2.3.5
only support MSVC_VERSION=12.0, which corresponds to Visual Studio
2013.

[[[
C:\Devel\serf_trunk>scons MSVC_VERSION=14.3
scons: Reading SConscript files ...

scons: warning: No version of Visual Studio compiler found - C/C++
compilers most likely not set correctly
File "C:\Devel\serf_trunk\SConstruct", line 27, in <module>
warning: replaced StringIO() for Python version < 3.
warning: replaced Conftest.CheckFunc() for SCons version < 4.7.

scons: warning: VC version 14.3 not installed.  C/C++ compilers are
most likely not set correctly.
 Installed versions are: []
]]]

I'm not able to test with Visual Studio 2013 since I don't have a
machine with that old version. Prior the change, SCons 2.3.5 would
have failed in the same way in my environment.

That said - I would not oppose requiring at least SCons >= 3.0.0. That
is the first version supporting Python 3 (and thus able to run on more
modern machines). It still supports 2.7 for those really requiring
something ancient. I'm not able to test anything older than 4.3.0 (on
Windows).

Cheers,
Daniel

Reply via email to