From 18b45e456412379c9182dd9cb4c8b30ca1e841b8 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 3 Jun 2024 07:18:54 -0600 Subject: [PATCH 1/6] Allow a Variable to not be substituted New parameter do_subst added to the variables Add method, if false indicates the variable value should not be substituted by the Variables logic. The default is True. Fixes #4241. Signed-off-by: Mats Wichmann --- CHANGES.txt | 3 +++ RELEASE.txt | 3 +++ SCons/Tool/yacc.xml | 4 ++-- SCons/Variables/PathVariable.py | 2 +- SCons/Variables/__init__.py | 26 ++++++++++++++++++-------- doc/generated/variables.gen | 4 ++-- doc/man/scons.xml | 17 ++++++++++++++++- 7 files changed, 45 insertions(+), 14 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ab13ef074b..8850bfe9d2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -85,6 +85,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER Now matches the annotation and docstring (which were prematurely updated in 4.6). All SCons usage except unit test was already fully consistent with a bool. + - When a variable is added to a Variables object, it can now be flagged + as "don't perform substitution". This allows variables to contain + characters which would otherwise cause expansion. Fixes #4241. RELEASE 4.7.0 - Sun, 17 Mar 2024 17:22:20 -0700 diff --git a/RELEASE.txt b/RELEASE.txt index b9c253f4d4..ec22f1057a 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -50,6 +50,9 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY Now matches the annotation and docstring (which were prematurely updated in 4.6). All SCons usage except unit test was already fully consistent with a bool. +- The Variables object Add method now accepts a do_subst keyword argument + (defaults to True) which can be set to inhibit substitution prior to + calling the variable's converter and validator. FIXES ----- diff --git a/SCons/Tool/yacc.xml b/SCons/Tool/yacc.xml index 82725dbade..729c408286 100644 --- a/SCons/Tool/yacc.xml +++ b/SCons/Tool/yacc.xml @@ -236,7 +236,7 @@ The value is used only if &cv-YACC_GRAPH_FILE_SUFFIX; is not set. The default value is .gv. -Changed in version 4.X.Y: deprecated. The default value +Changed in version 4.6.0: deprecated. The default value changed from .vcg (&bison; stopped generating .vcg output with version 2.4, in 2006). @@ -261,7 +261,7 @@ Various yacc tools have emitted various formats at different times. Set this to match what your parser generator produces. -New in version 4.X.Y. +New in version 4.6.0. diff --git a/SCons/Variables/PathVariable.py b/SCons/Variables/PathVariable.py index 6ea4e6bc93..4a827c5e12 100644 --- a/SCons/Variables/PathVariable.py +++ b/SCons/Variables/PathVariable.py @@ -141,7 +141,7 @@ def PathExists(key, val, env) -> None: # lint: W0622: Redefining built-in 'help' (redefined-builtin) def __call__( - self, key, help: str, default, validator: Optional[Callable] = None + self, key: str, help: str, default, validator: Optional[Callable] = None ) -> Tuple[str, str, str, Callable, None]: """Return a tuple describing a path list SCons Variable. diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py index 867493d7c8..03f7ef3b50 100644 --- a/SCons/Variables/__init__.py +++ b/SCons/Variables/__init__.py @@ -49,7 +49,7 @@ class Variable: """A Build Variable.""" - __slots__ = ('key', 'aliases', 'help', 'default', 'validator', 'converter') + __slots__ = ('key', 'aliases', 'help', 'default', 'validator', 'converter', 'do_subst') def __lt__(self, other): """Comparison fuction so Variable instances sort.""" @@ -87,9 +87,9 @@ def __init__( ) -> None: self.options: List[Variable] = [] self.args = args if args is not None else {} - if not SCons.Util.is_List(files): + if not SCons.Util.is_Sequence(files): files = [files] if files else [] - self.files = files + self.files: Sequence[str] = files self.unknown: Dict[str, str] = {} def __str__(self) -> str: @@ -132,6 +132,7 @@ def _do_add( option.default = default option.validator = validator option.converter = converter + option.do_subst = kwargs.get("subst", True) self.options.append(option) @@ -171,8 +172,11 @@ def Add( value before putting it in the environment. (default: ``None``) """ if SCons.Util.is_Sequence(key): - if not (len(args) or len(kwargs)): - return self._do_add(*key) + # If no other positional args (and no fundamental kwargs), + # unpack key, and pass the kwargs on: + known_kw = {'help', 'default', 'validator', 'converter'} + if not args and not known_kw.intersection(kwargs.keys()): + return self._do_add(*key, **kwargs) return self._do_add(key, *args, **kwargs) @@ -247,7 +251,10 @@ def Update(self, env, args: Optional[dict] = None) -> None: # apply converters for option in self.options: if option.converter and option.key in values: - value = env.subst(f'${option.key}') + if option.do_subst: + value = env.subst(f'${option.key}') + else: + value = env[option.key] try: try: env[option.key] = option.converter(value) @@ -262,7 +269,11 @@ def Update(self, env, args: Optional[dict] = None) -> None: # apply validators for option in self.options: if option.validator and option.key in values: - option.validator(option.key, env.subst(f'${option.key}'), env) + if option.do_subst: + value = env.subst('${%s}'%option.key) + else: + value = env[option.key] + option.validator(option.key, value, env) def UnknownVariables(self) -> dict: """Return dict of unknown variables. @@ -340,7 +351,6 @@ def GenerateHelpText(self, env, sort: Union[bool, Callable] = False) -> str: # removed so now we have to convert to a key. if callable(sort): options = sorted(self.options, key=cmp_to_key(lambda x, y: sort(x.key, y.key))) - elif sort is True: options = sorted(self.options) else: diff --git a/doc/generated/variables.gen b/doc/generated/variables.gen index 8c89616d35..fad7d5d4ae 100644 --- a/doc/generated/variables.gen +++ b/doc/generated/variables.gen @@ -10668,7 +10668,7 @@ Various yacc tools have emitted various formats at different times. Set this to match what your parser generator produces. -New in version 4.X.Y. +New in version 4.6.0. @@ -10826,7 +10826,7 @@ The value is used only if &cv-YACC_GRAPH_FILE_SUFFIX; is not set. The default value is .gv. -Changed in version 4.X.Y: deprecated. The default value +Changed in version 4.6.0: deprecated. The default value changed from .vcg (&bison; stopped generating .vcg output with version 2.4, in 2006). diff --git a/doc/man/scons.xml b/doc/man/scons.xml index cdaaa44ac9..eb02a23248 100644 --- a/doc/man/scons.xml +++ b/doc/man/scons.xml @@ -4835,7 +4835,7 @@ not to any stored-values files. - vars.Add(key, [help, default, validator, converter]) + vars.Add(key, [help, default, validator, converter, do_subst]) Add a customizable &consvar; to the &Variables; object. key @@ -4887,6 +4887,16 @@ or there is no separate validator it can raise a ValueError. + +Substitution will be performed on the variable value +as it is added, before the converter and validator are called, +unless the optional do_subst parameter +is false (default True). +Suppressing substitution may be useful if the variable value +looks like a &consvar; reference ($VAR) +to be expanded later. + + As a special case, if key is a sequence and is the only @@ -4919,6 +4929,11 @@ def valid_color(key, val, env): vars.Add('COLOR', validator=valid_color) + + +Changed in version 4.8.0: +added the do_subst parameter. + From 272d72beefb6b1d4efce68ec4c0ea2404f6fcb31 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 3 Jun 2024 10:38:21 -0600 Subject: [PATCH 2/6] Adding testcase for Add(..., subst) Signed-off-by: Mats Wichmann --- RELEASE.txt | 2 +- SCons/Variables/VariablesTests.py | 22 ++++++++++++++++++++++ SCons/Variables/__init__.py | 7 +++++-- doc/man/scons.xml | 12 ++++++------ 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/RELEASE.txt b/RELEASE.txt index ec22f1057a..063c116290 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -50,7 +50,7 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY Now matches the annotation and docstring (which were prematurely updated in 4.6). All SCons usage except unit test was already fully consistent with a bool. -- The Variables object Add method now accepts a do_subst keyword argument +- The Variables object Add method now accepts a subst keyword argument (defaults to True) which can be set to inhibit substitution prior to calling the variable's converter and validator. diff --git a/SCons/Variables/VariablesTests.py b/SCons/Variables/VariablesTests.py index 145bee31f3..866b2ff427 100644 --- a/SCons/Variables/VariablesTests.py +++ b/SCons/Variables/VariablesTests.py @@ -150,6 +150,28 @@ def test_Update(self) -> None: opts.Update(env, {}) assert env['ANSWER'] == 54 + # Test that the value is not substituted if 'subst' is False + def check_subst(key, value, env) -> None: + """Check that variable was not substituted before we get called.""" + assert value == "$ORIGIN", \ + f"Validator: '$ORIGIN' was substituted to {value!r}" + + def conv_subst(value) -> None: + """Check that variable was not substituted before we get called.""" + assert value == "$ORIGIN", \ + f"Converter: '$ORIGIN' was substituted to {value!r}" + return value + + opts.Add('NOSUB', + help='Variable whose value will not be substituted', + default='$ORIGIN', + validator=check_subst, + converter=conv_subst, + subst=False) + env = Environment() + opts.Update(env) + assert env['NOSUB'] == "$ORIGIN" + # Test that a bad value from the file is used and # validation fails correctly. test = TestSCons.TestSCons() diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py index 03f7ef3b50..80cda2b95b 100644 --- a/SCons/Variables/__init__.py +++ b/SCons/Variables/__init__.py @@ -114,6 +114,9 @@ def _do_add( """Create a Variable and add it to the list. Internal routine, not public API. + + .. versionadded:: 4.8.0 + *subst* keyword argument is now recognized. """ option = Variable() @@ -252,7 +255,7 @@ def Update(self, env, args: Optional[dict] = None) -> None: for option in self.options: if option.converter and option.key in values: if option.do_subst: - value = env.subst(f'${option.key}') + value = env.subst('${%s}' % option.key) else: value = env[option.key] try: @@ -270,7 +273,7 @@ def Update(self, env, args: Optional[dict] = None) -> None: for option in self.options: if option.validator and option.key in values: if option.do_subst: - value = env.subst('${%s}'%option.key) + value = env.subst('${%s}' % option.key) else: value = env[option.key] option.validator(option.key, value, env) diff --git a/doc/man/scons.xml b/doc/man/scons.xml index eb02a23248..57d38e8ee0 100644 --- a/doc/man/scons.xml +++ b/doc/man/scons.xml @@ -4835,7 +4835,7 @@ not to any stored-values files. - vars.Add(key, [help, default, validator, converter, do_subst]) + vars.Add(key, [help, default, validator, converter, subst]) Add a customizable &consvar; to the &Variables; object. key @@ -4889,12 +4889,12 @@ it can raise a ValueError. Substitution will be performed on the variable value -as it is added, before the converter and validator are called, -unless the optional do_subst parameter +before the converter and validator are called, +unless the optional subst parameter is false (default True). Suppressing substitution may be useful if the variable value -looks like a &consvar; reference ($VAR) -to be expanded later. +looks like a &consvar; reference (e.g. $VAR) +and the validator and/or converter should see it unexpanded. @@ -4932,7 +4932,7 @@ vars.Add('COLOR', validator=valid_color) Changed in version 4.8.0: -added the do_subst parameter. +added the subst parameter. From 91b6bcc272292756e47cee9548158cccfaff1755 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 3 Jun 2024 11:38:12 -0600 Subject: [PATCH 3/6] Additional testcase for subst on Variables This tests the other side of the coin: when vars.Add(..., subst=True) is used, substitution *is* performaed. Signed-off-by: Mats Wichmann --- SCons/Variables/VariablesTests.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/SCons/Variables/VariablesTests.py b/SCons/Variables/VariablesTests.py index 866b2ff427..7c6eaab171 100644 --- a/SCons/Variables/VariablesTests.py +++ b/SCons/Variables/VariablesTests.py @@ -151,26 +151,46 @@ def test_Update(self) -> None: assert env['ANSWER'] == 54 # Test that the value is not substituted if 'subst' is False - def check_subst(key, value, env) -> None: + # and that it is if 'subst' is True. + def check_no_subst(key, value, env) -> None: """Check that variable was not substituted before we get called.""" assert value == "$ORIGIN", \ f"Validator: '$ORIGIN' was substituted to {value!r}" - def conv_subst(value) -> None: + def conv_no_subst(value) -> None: """Check that variable was not substituted before we get called.""" assert value == "$ORIGIN", \ f"Converter: '$ORIGIN' was substituted to {value!r}" return value + def check_subst(key, value, env) -> None: + """Check that variable was substituted before we get called.""" + assert value == "Value", \ + f"Validator: '$SUB' was not substituted {value!r} instead of 'Value'" + + def conv_subst(value) -> None: + """Check that variable was not substituted before we get called.""" + assert value == "Value", \ + f"Converter: '$SUB' was substituted to {value!r} instead of 'Value'" + return value + opts.Add('NOSUB', help='Variable whose value will not be substituted', default='$ORIGIN', + validator=check_no_subst, + converter=conv_no_subst, + subst=False) + opts.Add('SUB', + help='Variable whose value will be substituted', + default='$VAR', validator=check_subst, converter=conv_subst, - subst=False) + subst=True) env = Environment() + env['VAR'] = "Value" opts.Update(env) - assert env['NOSUB'] == "$ORIGIN" + assert env['NOSUB'] == "$ORIGIN", env['NOSUB'] + assert env['SUB'] == env['VAR'], env['SUB'] # Test that a bad value from the file is used and # validation fails correctly. From a7f4ab10e2a20f15ef18db6bef9ed6b682468c55 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 3 Jun 2024 12:13:34 -0600 Subject: [PATCH 4/6] Small docstring update for vars.Add Signed-off-by: Mats Wichmann --- SCons/Variables/__init__.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py index 80cda2b95b..9c9c3f4961 100644 --- a/SCons/Variables/__init__.py +++ b/SCons/Variables/__init__.py @@ -135,7 +135,8 @@ def _do_add( option.default = default option.validator = validator option.converter = converter - option.do_subst = kwargs.get("subst", True) + option.do_subst = kwargs.pop("subst", True) + # TODO should any remaining kwargs be saved in the Variable? self.options.append(option) @@ -158,21 +159,26 @@ def Add( Arguments: key: the name of the variable, or a 5-tuple (or list). If *key* is a tuple, and there are no additional positional - arguments, it is unpacked into the variable name plus the four - listed keyword arguments from below. + arguments, it is unpacked into the variable name plus the + *help*, *default*, *validator* and *converter keyword args. If *key* is a tuple and there are additional positional arguments, the first word of the tuple is taken as the variable name, and the remainder as aliases. - args: optional positional arguments, corresponding to the four - listed keyword arguments. + args: optional positional arguments, corresponding to the + *help*, *default*, *validator* and *converter keyword args. kwargs: arbitrary keyword arguments used by the variable itself. Keyword Args: - help: help text for the variable (default: ``""``) + help: help text for the variable (default: empty string) default: default value for variable (default: ``None``) validator: function called to validate the value (default: ``None``) converter: function to be called to convert the variable's value before putting it in the environment. (default: ``None``) + subst: if true perform substitution on the value before the converter + and validator functions (if any) are called (default: ``True``) + + .. versionadded:: 4.8.0 + The *subst* keyword argument is now specially recognized. """ if SCons.Util.is_Sequence(key): # If no other positional args (and no fundamental kwargs), From 265046d40a5b4433bffd3d8da932ea5c7a4fb874 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 3 Jun 2024 12:30:40 -0600 Subject: [PATCH 5/6] Even more vars.Add() docstring tweaking. Signed-off-by: Mats Wichmann --- SCons/Variables/__init__.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/SCons/Variables/__init__.py b/SCons/Variables/__init__.py index 9c9c3f4961..34fb68b08d 100644 --- a/SCons/Variables/__init__.py +++ b/SCons/Variables/__init__.py @@ -113,7 +113,8 @@ def _do_add( ) -> None: """Create a Variable and add it to the list. - Internal routine, not public API. + This is the internal implementation for :meth:`Add` and + :meth:`AddVariables`. Not part of the public API. .. versionadded:: 4.8.0 *subst* keyword argument is now recognized. @@ -157,15 +158,16 @@ def Add( """Add a Build Variable. Arguments: - key: the name of the variable, or a 5-tuple (or list). - If *key* is a tuple, and there are no additional positional - arguments, it is unpacked into the variable name plus the - *help*, *default*, *validator* and *converter keyword args. - If *key* is a tuple and there are additional positional arguments, - the first word of the tuple is taken as the variable name, - and the remainder as aliases. + key: the name of the variable, or a 5-tuple (or other sequence). + If *key* is a tuple, and there are no additional arguments + except the *help*, *default*, *validator* and *converter* + keyword arguments, *key* is unpacked into the variable name + plus the *help*, *default*, *validator* and *converter* + arguments; if there are additional arguments, the first + elements of *key* is taken as the variable name, and the + remainder as aliases. args: optional positional arguments, corresponding to the - *help*, *default*, *validator* and *converter keyword args. + *help*, *default*, *validator* and *converter* keyword args. kwargs: arbitrary keyword arguments used by the variable itself. Keyword Args: @@ -174,7 +176,7 @@ def Add( validator: function called to validate the value (default: ``None``) converter: function to be called to convert the variable's value before putting it in the environment. (default: ``None``) - subst: if true perform substitution on the value before the converter + subst: perform substitution on the value before the converter and validator functions (if any) are called (default: ``True``) .. versionadded:: 4.8.0 From b78cfe95487433df7c9f3305571acab755fcdc1b Mon Sep 17 00:00:00 2001 From: William Deegan Date: Thu, 6 Jun 2024 18:37:12 +0200 Subject: [PATCH 6/6] added reference to new arg 'subst' in the blurb in CHANGES.txt --- CHANGES.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7fe6f8a517..66b149e203 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -86,8 +86,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER updated in 4.6). All SCons usage except unit test was already fully consistent with a bool. - When a variable is added to a Variables object, it can now be flagged - as "don't perform substitution". This allows variables to contain - characters which would otherwise cause expansion. Fixes #4241. + as "don't perform substitution" by setting the argument subst. + This allows variables to contain characters which would otherwise + cause expansion. Fixes #4241. - The test runner now recognizes the unittest module's return code of 5, which means no tests were run. SCons/Script/MainTests.py currently has no tests, so this particular error code is expected - should not