Updates on Value nodes

Some doc changes.
E2E tests cleaned up a bit.

Before merge:
TODO: second half of manpage example doesn't work.

Also: should test give SCons.Node.Value.Value as factory, or just Value?
  (remove new import if the latter).  SCons.Node.Value.Value and
  SCons.Environment.Value are not the same thing.

Signed-off-by: Mats Wichmann <mats@linux.com>

9 files changed, 104 insertions, 79 deletions

diff --git a/.appveyor.yml b/.appveyor.yml
index f7e23f6..07b6fe1 100644
--- a/.appveyor.yml
+++ b/.appveyor.yml
@@ -36,7 +36,7 @@ environment:
   matrix:
     # Test oldest and newest supported Pythons, and a subset in between.
     # Skipping 3.7 and 3.9 at this time
-    - WINPYTHON: "Python311"
+    - WINPYTHON: "Python312"
 
     - WINPYTHON: "Python310"
 
@@ -50,7 +50,7 @@ matrix:
   exclude:
     # test python 3.6 on Visual Studio 2017 image
     - image: Visual Studio 2017
-      WINPYTHON: "Python311"
+      WINPYTHON: "Python312"
     - image: Visual Studio 2017
       WINPYTHON: "Python310"
     - image: Visual Studio 2017
@@ -58,7 +58,7 @@ matrix:
 
     # test python 3.8 on Visual Studio 2019 image
     - image: Visual Studio 2019
-      WINPYTHON: "Python311"
+      WINPYTHON: "Python312"
     - image: Visual Studio 2019
       WINPYTHON: "Python310"
     - image: Visual Studio 2019
diff --git a/CHANGES.txt b/CHANGES.txt
index f1e9117..0f67958 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -104,6 +104,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       DeprecatedMissingSConscriptWarning) add two warnings to manpage
       (cache-cleanup-error, future-reserved-variable), improve unittest, tweak
       Sphinx build.
+    - Updated Value Node docs and tests.
 
 
 RELEASE 4.6.0 -  Sun, 19 Nov 2023 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index b339c44..24c68ab 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -96,6 +96,7 @@ DOCUMENTATION
 - The manpage entry for SharedLibrary was clarified.
 - Update API docs for Warnings framework; add two warns to manpage
   enable/disable control.
+- Updated Value Node docs.
 
 DEVELOPMENT
 -----------
diff --git a/SCons/Environment.xml b/SCons/Environment.xml
index 0014426..e4ade51 100644
--- a/SCons/Environment.xml
+++ b/SCons/Environment.xml
@@ -3511,33 +3511,22 @@ gltool(env)  # adds 'opengl' to the TOOLS variable
 </arguments>
 <summary>
 <para>
-Returns a Node object representing the specified &Python; value.  Value
-Nodes can be used as dependencies of targets.  If the result of
-calling
-<function>str</function>(<parameter>value</parameter>)
-changes between SCons runs, any targets depending on
-<function>Value</function>(<parameter>value</parameter>)
-will be rebuilt.
-(This is true even when using timestamps to decide if
-files are up-to-date.)
-When using timestamp source signatures, Value Nodes'
-timestamps are equal to the system time when the Node is created.
+Returns a Node object representing the specified &Python;
+<parameter>value</parameter>.
+Value Nodes can be used as dependencies of targets.
+If the string representation of the Value Node
+changes between &SCons; runs, it is considered
+out of date and any targets depending it will be rebuilt.
+Since Value Nodes have no filesystem representation,
+timestamps are not used; the timestamp deciders
+perform the same content-based up to date check.
 </para>
-
 <para>
-The returned Value Node object has a
-<function>write</function>()
-method that can be used to "build" a Value Node
-by setting a new value.
 The optional
 <parameter>built_value</parameter>
 argument can be specified
 when the Value Node is created
-to indicate the Node should already be considered
-"built."
-There is a corresponding
-<function>read</function>()
-method that will return the built value of the Node.
+to indicate the Node should already be considered "built."
 </para>
 
 <para>
@@ -3548,6 +3537,16 @@ cannot be converted to a string.
 </para>
 
 <para>
+Value Nodes have a
+<methodname>write</methodname>
+method that can be used to "build" a Value Node
+by setting a new value.
+The corresponding
+<methodname>read</methodname>
+method returns the built value of the Node.
+</para>
+
+<para>
 <emphasis>Changed in version 4.0:</emphasis>
 the <parameter>name</parameter> parameter was added.
 </para>
@@ -3556,36 +3555,39 @@ the <parameter>name</parameter> parameter was added.
 Examples:
 </para>
 
+<!-- TODO fix second example or replace - still doesn't work -->
 <example_commands>
 env = Environment()
 
 def create(target, source, env):
-    # A function that will write a 'prefix=$SOURCE'
-    # string into the file name specified as the
-    # $TARGET.
+    """Action function to create a file from a Value.
+
+    Writes 'prefix=$SOURCE' into the file name given as $TARGET.
+    """
     with open(str(target[0]), 'wb') as f:
-        f.write('prefix=' + source[0].get_contents())
+        f.write(b'prefix=' + source[0].get_contents() + b'\n')
 
-# Fetch the prefix= argument, if any, from the command
-# line, and use /usr/local as the default.
+# Fetch the prefix= argument, if any, from the command line.
+# Use /usr/local as the default.
 prefix = ARGUMENTS.get('prefix', '/usr/local')
 
-# Attach a .Config() builder for the above function action
-# to the construction environment.
+# Attach builder named Config to the construction environment
+# using the 'create' action function above.
 env['BUILDERS']['Config'] = Builder(action=create)
 env.Config(target='package-config', source=Value(prefix))
 
 def build_value(target, source, env):
-    # A function that "builds" a Python Value by updating
-    # the Python value with the contents of the file
-    # specified as the source of the Builder call ($SOURCE).
+    """Action function to "build" a Value.
+
+    Writes contents of $SOURCE into $TARGET, thus updating if it existed.
+    """
     target[0].write(source[0].get_contents())
 
 output = env.Value('before')
 input = env.Value('after')
 
-# Attach a .UpdateValue() builder for the above function
-# action to the construction environment.
+# Attach a builder named UpdateValue to the construction environment
+# using the 'build_value' action function above.
 env['BUILDERS']['UpdateValue'] = Builder(action=build_value)
 env.UpdateValue(target=Value(output), source=Value(input))
 </example_commands>
@@ -3600,7 +3602,7 @@ env.UpdateValue(target=Value(output), source=Value(input))
 <para>
 Sets up a mapping to define a variant build directory in
 <parameter>variant_dir</parameter>.
-<parameter>src_dir</parameter> may not be underneath
+<parameter>src_dir</parameter> must not be underneath
 <parameter>variant_dir</parameter>.
 A &f-VariantDir; mapping is global, even if called using the
 &f-env-VariantDir; form.
diff --git a/SCons/Node/PythonTests.py b/SCons/Node/PythonTests.py
index 2be2b29..95ea0d5 100644
--- a/SCons/Node/PythonTests.py
+++ b/SCons/Node/PythonTests.py
@@ -25,12 +25,12 @@ import unittest
 
 import SCons.Errors
 import SCons.Node.Python
+from SCons.Script import Depends
 
 
 class ValueTestCase(unittest.TestCase):
     def test_Value(self) -> None:
-        """Test creating a Value() object
-        """
+        """Test creating a Value() object."""
         v1 = SCons.Node.Python.Value('a')
         assert v1.value == 'a', v1.value
 
@@ -39,45 +39,50 @@ class ValueTestCase(unittest.TestCase):
         assert v2.value == value2, v2.value
         assert v2.value is value2, v2.value
 
+        # the two nodes are not the same though they have same attributes
         assert v1 is not v2
         assert v1.value == v2.value
+        assert v1.name == v2.name
 
+        # node takes the built_value if one is supplied.
         v3 = SCons.Node.Python.Value('c', 'cb')
         assert v3.built_value == 'cb'
 
     def test_build(self) -> None:
-        """Test "building" a Value Node
-        """
+        """Test "building" a Value Node."""
         class fake_executor:
             def __call__(self, node) -> None:
                 node.write('faked')
 
-        v1 = SCons.Node.Python.Value('b', 'built')
+        # *built_value* arg means already built, executor will not be called
+        v1 = SCons.Node.Python.Value('b', built_value='built')
         v1.executor = fake_executor()
         v1.build()
         assert v1.built_value == 'built', v1.built_value
 
+        # not built, executor will build it
         v2 = SCons.Node.Python.Value('b')
         v2.executor = fake_executor()
         v2.build()
         assert v2.built_value == 'faked', v2.built_value
 
+        # test the *name* parameter to refer to the node
         v3 = SCons.Node.Python.Value(b'\x00\x0F', name='name')
         v3.executor = fake_executor()
         v3.build()
-        assert v3.name == 'name', v3.name
         assert v3.built_value == 'faked', v3.built_value
+        # building the node does not change the name
+        assert v3.name == 'name', v3.name
 
     def test_read(self) -> None:
-        """Test the Value.read() method
-        """
+        """Test the Value.read() method."""
         v1 = SCons.Node.Python.Value('a')
         x = v1.read()
         assert x == 'a', x
 
     def test_write(self) -> None:
-        """Test the Value.write() method
-        """
+        """Test the Value.write() method."""
+        # creating the node without built_value does not set it
         v1 = SCons.Node.Python.Value('a')
         assert v1.value == 'a', v1.value
         assert not hasattr(v1, 'built_value')
@@ -87,8 +92,7 @@ class ValueTestCase(unittest.TestCase):
         assert v1.built_value == 'new', v1.built_value
 
     def test_get_csig(self) -> None:
-        """Test calculating the content signature of a Value() object
-        """
+        """Test calculating the content signature of a Value() object."""
         v1 = SCons.Node.Python.Value('aaa')
         csig = v1.get_csig(None)
         assert csig == 'aaa', csig
@@ -101,8 +105,17 @@ class ValueTestCase(unittest.TestCase):
         csig = v3.get_csig(None)
         assert csig == 'None', csig
 
-
-
+        # Dependencies: a tree of Value nodes comes back as a single string.
+        # This may change someday, bot for now:
+        v1 = SCons.Node.Python.Value('node1')
+        v2 = SCons.Node.Python.Value('node2')
+        v3 = SCons.Node.Python.Value('node3')
+        v4 = SCons.Node.Python.Value('node4')
+        Depends(v1, [v2, v3])
+        Depends(v3, v4)
+        assert v1.read() == 'node1', v1.read
+        csig = v1.get_csig()
+        assert csig == 'node1node2node3node4', csig
 
 
 class ValueNodeInfoTestCase(unittest.TestCase):
diff --git a/SCons/Taskmaster/Job.py b/SCons/Taskmaster/Job.py
index 73ec0df..fae985e 100644
--- a/SCons/Taskmaster/Job.py
+++ b/SCons/Taskmaster/Job.py
@@ -278,9 +278,8 @@ class ThreadPool:
 
         try:
             prev_size = threading.stack_size(stack_size * 1024)
-        except AttributeError as e:
-            # Only print a warning if the stack size has been
-            # explicitly set.
+        except RuntimeError as e:
+            # Only print a warning if the stack size has been explicitly set.
             if explicit_stack_size is not None:
                 msg = "Setting stack size is unsupported by this version of Python:\n    " + \
                     e.args[0]
diff --git a/test/Actions/function.py b/test/Actions/function.py
index 699cd17..0b191e4 100644
--- a/test/Actions/function.py
+++ b/test/Actions/function.py
@@ -39,9 +39,6 @@ test = TestSCons.TestSCons()
 test.write('SConstruct', r"""
 import re
 
-import SCons.Action
-import SCons.Builder
-
 options = Variables()
 options.AddVariables(
     ('header', 'Header string (default cell argument)', 'Head:'),
@@ -91,8 +88,8 @@ def toto(header='%(header)s', trailer='%(trailer)s'):
 
 exec(withClosure % optEnv)
 
-genHeaderBld = SCons.Builder.Builder(
-    action=SCons.Action.Action(toto(), 'Generating $TARGET', varlist=['ENVDEPS']),
+genHeaderBld = Builder(
+    action=Action(toto(), 'Generating $TARGET', varlist=['ENVDEPS']),
     suffix='.gen.h',
 )
 
diff --git a/test/Value/GetContent.py b/test/Value/GetContent.py
index 8fbbf29..30e30df 100644
--- a/test/Value/GetContent.py
+++ b/test/Value/GetContent.py
@@ -1,6 +1,8 @@
 #!/usr/bin/env python
 #
-# __COPYRIGHT__
+# SPDX-License-Identifier: MIT
+#
+# Copyright The SCons Foundation
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and associated documentation files (the
@@ -20,9 +22,6 @@
 # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-#
-
-__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 """
 Test the Value node as a build target
@@ -33,18 +32,25 @@ import TestSCons
 test = TestSCons.TestSCons()
 
 test.write('SConstruct', """
-import SCons.Script
+import SCons.Node
+
 def null_build(target, source, env):
     pass
+
 env = DefaultEnvironment()
-env['BUILDERS']['ValueBuilder'] =  SCons.Builder.Builder(
-    action=SCons.Action.Action(null_build),
-    target_factory=SCons.Node.Python.Value,    
-)    
-v = env.ValueBuilder("myvalue",env.Dir("#"))
-v[0].get_text_contents()
+env['BUILDERS']['ValueBuilder'] = Builder(
+    action=Action(null_build),
+    target_factory=SCons.Node.Python.Value,
+)
+v = env.ValueBuilder("myvalue", env.Dir("#"))
+_ = v[0].get_text_contents()  # only care it doesn't take exception
 """)
 
 test.run()
 test.pass_test()
 
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4:
diff --git a/test/Value.py b/test/Value/Value.py
index 78cd152..7bb48ca 100644
--- a/test/Value.py
+++ b/test/Value/Value.py
@@ -23,6 +23,8 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+"""Test Value() with different deciders."""
+
 import re
 
 import TestSCons
@@ -34,12 +36,16 @@ test = TestSCons.TestSCons(match=TestCmd.match_re)
 
 python = TestSCons.python
 
+# do not f-string, "source_signature" substituted in a loop below
 SConstruct_content = """
 Decider(r'%(source_signature)s')
 
 class Custom:
-    def __init__(self, value):  self.value = value
-    def __str__(self):          return "C=" + str(self.value)
+    def __init__(self, value):
+        self.value = value
+
+    def __str__(self):
+        return "C=" + str(self.value)
 
 P = ARGUMENTS.get('prefix', '/usr/local')
 L = len(P)
@@ -51,22 +57,22 @@ def create(target, source, env):
 
 DefaultEnvironment(tools=[])  # test speedup
 env = Environment()
-env['BUILDERS']['B'] = Builder(action = create)
-env['BUILDERS']['S'] = Builder(action = r'%(_python_)s put.py $SOURCES into $TARGET')
+env['BUILDERS']['B'] = Builder(action=create)
+env['BUILDERS']['S'] = Builder(action=r'%(_python_)s put.py $SOURCES into $TARGET')
 env.B('f1.out', Value(P))
 env.B('f2.out', env.Value(L))
 env.B('f3.out', Value(C))
 env.S('f4.out', Value(L))
 
-def create_value (target, source, env):
+def create_value(target, source, env):
     target[0].write(source[0].get_contents())
 
-def create_value_file (target, source, env):
+def create_value_file(target, source, env):
     with open(str(target[0]), 'wb') as f:
         f.write(source[0].read())
 
-env['BUILDERS']['B2'] = Builder(action = create_value)
-env['BUILDERS']['B3'] = Builder(action = create_value_file)
+env['BUILDERS']['B2'] = Builder(action=create_value)
+env['BUILDERS']['B3'] = Builder(action=create_value_file)
 
 V = Value('my value')
 env.B2(V, 'f3.out')