Merge pull request SCons#4550 from mwichmann/variables-nosubst

Allow a Variable to not be substituted

Author: bdbaddog

CHANGES.txt

@@ -85,6 +85,10 @@ 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" 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

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 subst keyword argument
+  (defaults to True) which can be set to inhibit substitution prior to
+  calling the variable's converter and validator.
 
 FIXES
 -----

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 <filename>.gv</filename>.
 </para>
 <para>
-<emphasis>Changed in version 4.X.Y</emphasis>: deprecated. The default value
+<emphasis>Changed in version 4.6.0</emphasis>: deprecated. The default value
 changed from <filename>.vcg</filename> (&bison; stopped generating
 <filename>.vcg</filename> output with version 2.4, in 2006).
 </para>
@@ -261,7 +261,7 @@ Various yacc tools have emitted various formats
 at different times.
 Set this to match what your parser generator produces.
 </para>
-<para><emphasis>New in version 4.X.Y</emphasis>. </para>
+<para><emphasis>New in version 4.6.0</emphasis>. </para>
 </summary>
 </cvar>
 

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.
 

SCons/Variables/VariablesTests.py

@@ -150,6 +150,48 @@ def test_Update(self) -> None:
         opts.Update(env, {})
         assert env['ANSWER'] == 54
 
+        # Test that the value is not substituted if 'subst' is False
+        # 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_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=True)
+        env = Environment()
+        env['VAR'] = "Value"
+        opts.Update(env)
+        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.
         test = TestSCons.TestSCons()

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:
@@ -113,7 +113,11 @@ 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.
         """
         option = Variable()
 
@@ -132,6 +136,8 @@ def _do_add(
         option.default = default
         option.validator = validator
         option.converter = converter
+        option.do_subst = kwargs.pop("subst", True)
+        # TODO should any remaining kwargs be saved in the Variable?
 
         self.options.append(option)
 
@@ -152,27 +158,36 @@ 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 four
-            listed keyword arguments from below.
-            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.
+          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.
           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: 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 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 +262,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('${%s}' % option.key)
+                else:
+                    value = env[option.key]
                 try:
                     try:
                         env[option.key] = option.converter(value)
@@ -262,7 +280,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 +362,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:

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.
 </para>
-<para><emphasis>New in version 4.X.Y</emphasis>. </para>
+<para><emphasis>New in version 4.6.0</emphasis>. </para>
 </listitem>
   </varlistentry>
   <varlistentry id="cv-YACC_HEADER_FILE">
@@ -10826,7 +10826,7 @@ The value is used only if &cv-YACC_GRAPH_FILE_SUFFIX; is not set.
 The default value is <filename>.gv</filename>.
 </para>
 <para>
-<emphasis>Changed in version 4.X.Y</emphasis>: deprecated. The default value
+<emphasis>Changed in version 4.6.0</emphasis>: deprecated. The default value
 changed from <filename>.vcg</filename> (&bison; stopped generating
 <filename>.vcg</filename> output with version 2.4, in 2006).
 </para>

doc/man/scons.xml

@@ -4835,7 +4835,7 @@ not to any stored-values files.
 
 <variablelist>
   <varlistentry id="v-Add">
-  <term><replaceable>vars</replaceable>.<function>Add</function>(<parameter>key, [help, default, validator, converter]</parameter>)</term>
+  <term><replaceable>vars</replaceable>.<function>Add</function>(<parameter>key, [help, default, validator, converter, subst]</parameter>)</term>
   <listitem>
 <para>Add a customizable &consvar; to the &Variables; object.
 <parameter>key</parameter>
@@ -4887,6 +4887,16 @@ or there is no separate validator
 it can raise a <exceptionname>ValueError</exceptionname>.
 </para>
 
+<para>
+Substitution will be performed on the variable value
+before the converter and validator are called,
+unless the optional <parameter>subst</parameter> parameter
+is false (default <literal>True</literal>).
+Suppressing substitution may be useful if the variable value
+looks like a &consvar; reference (e.g. <literal>$VAR</literal>)
+and the validator and/or converter should see it unexpanded.
+</para>
+
 <para>
 As a special case, if <parameter>key</parameter>
 is a sequence and is the <emphasis>only</emphasis>
@@ -4919,6 +4929,11 @@ def valid_color(key, val, env):
 
 vars.Add('COLOR', validator=valid_color)
 </programlisting>
+
+<para>
+<emphasis>Changed in version 4.8.0:</emphasis>
+added the <parameter>subst</parameter> parameter.
+</para>
   </listitem>
   </varlistentry>