Change Variables() such that you can disable subst()'ing before running converter or validator

[bdbaddog]

Currently, the logic for Variables will subst the passed value for a variable before running the converter and validator (per each if specified). This isn't always desirable. Suggest add a flag or other way to disable one or both of those substs

Required information

• Version of SCons 4.4.0
• Version of Python N/A

Discord discussion: https://discord.com/channels/571796279483564041/1029498487156527114

[mwichmann]

RFI: I can't follow that link (get lost in the morass of trying to vector through the web browser to authenticate before being allowed to see, which no longer works for me), could we add a detail for

This isn't always desirable

[bdbaddog]

Here's the discord thread content:

acm — 10/11/2022 1:59 PM Does python come with something builtin to do "array literal in a string to array"?

bdbaddog — 10/11/2022 2:18 PM str.split() ?

bdbaddog — 10/11/2022 2:35 PM so I'd be if it hadn't initially been named Options() ListVariable would have been named OptionsVariable() which is actually a better name, then ListVariable() could be used for this.. but is your query only about passing a variable on command line which would have file paths?

acm — 10/11/2022 2:44 PM Not just file paths The problem is any "list-y" thing Not all lists are paths Probably many are, but I wouldn't want to require that I think the naming stuff is not really relevant

bdbaddog — 10/11/2022 2:49 PM the naming? you mean OptionsVariable(),etc.. (just thinking out loud there, not meant to address your query) I think we want variable evaluation to have an option to not subst if there is a converter.

acm — 10/11/2022 2:51 PM There are some other oddities in validator and converter Like if you have: Variable(validator=validate, converter=int()) The validator gets called with a string It is a little puzzling So if you have a flag that can only take the integers 4 or 5, your validator has to validate '4' and '5' And I agree it is weird that having a converter implies an unavoidable subst I got stuck on RPATH when I tried this in a variables file: RPATH=[SCons.Subst.Literal('$ORIGIN/../lib')]

That would have worked, except that RPATH has a converter So it gets substed.

bdbaddog — 10/11/2022 3:02 PM Do you think you'd always want to either subst for both converter and validator, or for neither? or selectively for each?

acm — 10/11/2022 3:04 PM Yeah I really don't know I can see reasons for doing it both ways I'll think about it I will probably need to end this bit of yak shaving and go find a solution that does not involve setting an $ORIGIN -y RPATH on the command line.

bdbaddog — 10/11/2022 3:06 PM that for linux linker? mac's different right? for your usage you'd either want RPATH or not right? if so the value would be always $ORIGIN/../lib ?

bdbaddog — 10/11/2022 3:25 PM Added this to add details and not drop this. https://github.com/SCons/scons/issues/4241

[mwichmann]

that's useful, thanks. Somehow I'm not surprised the always beloved \\\\\\\\\$ORIGIN is involved here.... that one always causes trouble, it seems :-)

[mwichmann]

Okay, to pick this bit:

Like if you have Variable(validator=validate, converter=int()), the validator gets called with a string. It is a little puzzling. So if you have a flag that can only take the integers 4 or 5, your validator has to validate '4' and '5'

The design overall is a little awkward. Despite what the manpage says, the rough order is:

• make sure the input sources are in a dict
• copy the k:v pairs from the dict to env
• call the converter on each option that was added. May cause env[key] to be updated. For ListVariable in particular, it changes from a string to an instance of a private class; for the cited example, it changed to an int.
• call the validator on each option that was added, giving a chance to bail if something doesn't mesh.

Calling subst before passing to the validator is how it fetches the updated value saved by calling the converter, but you could just get the raw value via env[key] without doing the subst. I, too, find it a little odd that you let the converter change things, and then you force the result back into a string, but I guess it saves each individual validator from doing its own subst and so is a little more concise?

Is there something actionable in this issue?

[bdbaddog]

Sure. I think a subst=True/False on Variable, with default of True would work?

[mwichmann]

Yeah, probably. After "sleeping on it", I don't get the value of subst'ing a second time, before calling the validator. You've done substitution before calling the converter, which then converts to the right form. Turning it back into a string for the validator just doesn't seem right, unless you believe everything should be a string (if it's not a bool). The docs don't say that is the case, but I'm suspecting that was the thinking during development. For example, the unit test for EnumVariable (and the e2e test as well) test only string values, never, for example, ints.

subst'ing before the converter makes a little more sense: variables coming from the command line are always strings, but the input source can also be python expressions in a file and/or an arbritrary dict, where the values could be almost anything. subst'ing makes the converters simple: get a string, do whatever transformations you need on that; you don't have to first stop and check what kind of data you got. But then you get the case of this issue: the subst was perhaps only intended to cover getting the value from the env, but subst recurses, so if the value accidentally looked like a $VAR, that will be replaced, too. Not sure how much sense that makes.

[mwichmann]

And... a quick check shows that the tests do expect the value to be subst'd, specifically for PathVariable, it even has cascading variables in one case,

    PathVariable('qtdir', 'where the root of Qt is installed', qtdir),         
    PathVariable('qt_libraries', 'where the Qt library is installed', os.path.join('$qtdir', 'lib'))

But it's not hard for validators that need substitution to do it themselves, if one wanted to go that way.

Edit: incomplete info. PathVariable has this specific issue because there's no converter for PathVariable. So it never gets the benfit of the "first subst".

[mwichmann]

Going back to an earlier comment:

Sure. I think a subst=True/False on Variable, with default of True would work?

This is easy to do - I have a prototype - but after reading an article entitled roughly "The Boolean Trap", one wonders if a string value is better, to allow for possible future flexibility. One could imagine, without having use cases in hand, that controlling whether both converter and validator subst or not might be interesting; or the type of subst (the subst module intself defines those three styles - SUBST_SIG, SUBST_CMD and SUBST_RAW). Maybe that's all just YAGNI, but is a bool unnecessarily restrictive?

More importantly: is this actually worth doing?

[bdbaddog]

I think bool is sufficient. If we really found we needed more flexibility then we could complicate the solution more.. I think it's worth doing, it could remove some fairly obnoxious hacks needed to avoid the subst'ing when it's not desired.