SCons
Fix ListVariable with a space-containing value
A call like: scons SKIPUTILS="NSIS Menu" for ListVariable SKIPUTILS, with a space embedded in the value and thus given quoted, did not work, as the splitting of ListVariable into separate converter / validator routines left subst() running twice. The call before the converter is invoked was fine; but the converter turns the value into an instance of the internal list-variable class, which is then stored, and running subst() on that before the validator was invoked undid that work and turned it back into a string - which the validator then split incorrectly because of the lost context. There's no completely clean fix (perhaps other than going back to a combined converter/validator like before 4.8.0). The approach chosen was to not run the subst before the validator if the stored value doesn't look like a string - indicating it's probably already in the form we wanted.
(Edited) Testing this was complicated by the test framework it splits the arguments= parameter, and so breaks with embedded spaces like test.run(arguments='TEST="a b"'). Attempts to fix this up just created more problems, so the test additions now use a list to supply strings that should not be split, like arguments=['TEST="a', 'b"']. There's plenty of precedent for this, many tests are run this way without necessarily even needing to be. Added some doc notes on this. The various variables are now tested for space-included strings, which can be values in an Enum or List type, or path strings (think Windows paths...) in Path and Package types.
Although not directly related to the space problem, during the investigation it also became clear that the new option to avoid calling subst() before the converter (the earlier fix for #4241) could have side effects - it is now possible for a variable's converter to be called with a type that is not an expected user input type. This could happen with BoolVariable and PathVariable: here the inputs were all expected to be strings, but the default to use if the variable is not specified could be in a "final" type - a bool (for PathVariable it's bool-or-str). If subst is used, those will arrive as stringified versions of the bool, but if subst is suppressed they will actually be bool, so those converters needed to check for a bool type before doing a string.lower().
Signed-off-by: Mats Wichmann [email protected]
Contributor Checklist:
- [X] I have created a new test or updated the unit tests to cover the new/changed functionality.
- [X] I have updated
CHANGES.txt(and read theREADME.rst) - [X] I have updated the appropriate documentation
