ListVariable with space parsing broke in 4.8.0

[kichik]

Our NSIS build broke with 4.8.0: https://github.com/kichik/nsis/actions/runs/10330863317/job/28603274986
Works with SCons 4.7.0: https://github.com/kichik/nsis/actions/runs/9660627425/job/26646616162

The issue can be boiled down to the following:

env = Environment()
opts = Variables()
opts.Add(ListVariable('TEST', 'test', 'none', ['a b', 'c d', 'e']))
opts.Update(env)
print(env["TEST"])

With SCons 4.7.0:

# scons TEST="a b"
scons: Reading SConscript files ...
a b
scons: done reading SConscript files.
scons: Building targets ...
scons: `.' is up to date.
scons: done building targets.

But with SCons 4.8.0:

# scons TEST="a b"
scons: Reading SConscript files ...

scons: *** Invalid value(s) for variable 'TEST': 'a,b'. Valid values are: a b,c d,e,all,none
File "/tmp/a/SConstruct", line 4, in <module>

Required information

• Link to SCons Discord thread discussing your issue
• SCons 4.8.0
• CPython 3.12.3 on Linux but can be reproduced with any version and any OS
• SCons was installed with pip, brew, venv, anything...

[mwichmann]

Yes, I can confirm the problem. This was introduced when the validation logic was split from the conversion logic in ListVariable... which may help explain why they were kept together in the first place. In the Variables.Update method, it loops through all the options, subst'ing the value if needed, then calling the converter and storing the result. Then it loops through all the options again, subst'ing the (previously stored) value if needed, then calling the validator and bailing out if there's a problem. Unfortunately, a ListOption doesn't really survive running subst the second time - the converter has already put the value in the right form, a list of strings, thus preserving any embedded spaces, feeding that to subst again turns it back into a string which later gets split. There wasn't a test that tickled this.

A couple of easy approaches break other things - there are a couple of Variables tests that actually depend on the subst before the validator, it turns out.

As an irritating additional problem, the test framework has a bug where it doesn't preserve quoted substrings in the arguments to the instance of SCons it invokes (despite having a piece of code with a comment claiming to "preserve spaces"), so while the CLI sequence scons TEST="a b" delivers the right thing to the Variables code, having a test case do test.run(arguments='TEST="A B"') does not let SCons see a correct command line. I have a fix for that (hopefully preserving what's left of my sanity).