Ensure get_contents() method of Node objects return 'bytes' and fix decoding errors in Value.get_csig()

CHANGES.txt

@@ -11,6 +11,15 @@ RELEASE 4.1.0.devyyyymmdd - Mon, 04 Jul 2020 16:06:40 -0700
       From John Doe:
 
         - Whatever John Doe did.
+
+  From Eugene Huang:
+    - Ensure all Node subclasses return 'bytes' in their get_contents()
+      methods.
+    - Make Value.get_text_contents() calculate the contents using child node
+      csigs instead of child contents (similar to Alias node contents). The
+      new implementation is not prone to decoding errors so Value.get_csig()
+      should always work, whereas previously, Value nodes sometimes could not
+      be used as target nodes.
 
 
 RELEASE 4.0.0 - Sat, 04 Jul 2020 12:00:27 +0000

SCons/Node/Python.py

@@ -138,24 +138,20 @@ def read(self):
     def get_text_contents(self):
         """By the assumption that the node.built_value is a
         deterministic product of the sources, the contents of a Value
-        are the concatenation of all the contents of its sources.  As
-        the value need not be built when get_contents() is called, we
+        are the concatenation of all the content signatures of its sources.
+        As the value need not be built when get_contents() is called, we
         cannot use the actual node.built_value."""
-        ###TODO: something reasonable about universal newlines
         contents = str(self.value)
-        for kid in self.children(None):
-            contents = contents + kid.get_contents().decode()
+        contents += ''.join(n.get_csig() for n in self.children())
         return contents
 
     def get_contents(self):
         """
-        Get contents for signature calculations.
+        Same as get_text_contents() but encode as utf-8 in case other
+        functions rely on get_contents() existing.
         :return: bytes
         """
-        contents = str(self.value).encode('utf-8')
-        for kid in self.children(None):
-            contents = contents + kid.get_contents()
-        return contents
+        return self.get_text_contents().encode('utf-8')
 
     def changed_since_last_build(self, target, prev_ni):
         cur_csig = self.get_csig()
@@ -176,10 +172,9 @@ def get_csig(self, calc=None):
         except AttributeError:
             pass
 
-        contents = self.get_contents().decode(errors='backslashreplace')
+        csig = self.get_text_contents()
 
-        self.get_ninfo().csig = contents
-        return contents
+        return csig
 
 
 def ValueWithMemo(value, built_value=None, name=None):

SCons/Node/PythonTests.py

@@ -103,39 +103,58 @@ def test_get_csig(self):
         csig = v3.get_csig(None)
         assert csig == 'None', csig
 
-    def test_get_content_with_child_binary_content(self):
+    def test_get_csig_with_children(self):
+        """Test calculating the content signature of a Value() object with child
+        nodes.
+        """
         class DummyNode:
-            def __init__(self, contents):
-                self.contents = contents
-            def get_contents(self):
-                return self.contents
+            def __init__(self, csig):
+                self.csig = csig
+            def get_csig(self, calc=None):
+                return self.csig
 
-        # Node with binary content that is not valid utf-8.
-        node_with_binary = DummyNode(b'\xff')
+        v = SCons.Node.Python.Value('aaa')
 
-        v = SCons.Node.Python.Value('v')
-        v.add_dependency([node_with_binary])
+        c1 = DummyNode(csig='csig1')
+        c2 = DummyNode(csig='csig2')
 
-        # Just make sure this call doesn't fail. Not sure what to check the
-        # return value against.
-        v.get_contents()
+        v.add_dependency([c1, c2])
+        csig = v.get_csig(None)
+        assert csig == 'aaacsig1csig2', csig
 
-    def test_get_csig_with_child_binary_content(self):
+    def test_get_text_contents_with_children(self):
+        """Test calculating text contents with child nodes
+        """
         class DummyNode:
-            def __init__(self, contents):
-                self.contents = contents
-            def get_contents(self):
-                return self.contents
+            def __init__(self, csig):
+                self.csig = csig
+            def get_csig(self):
+                return self.csig
 
-        # Node with binary content that is not valid utf-8.
-        node_with_binary = DummyNode(b'\xff')
+        v = SCons.Node.Python.Value('aaa')
+        c1 = DummyNode(csig='csig1')
+        c2 = DummyNode(csig='csig2')
 
-        v = SCons.Node.Python.Value('v')
-        v.add_dependency([node_with_binary])
+        v.add_dependency([c1, c2])
+        text_contents = v.get_text_contents()
+        assert text_contents == 'aaacsig1csig2', text_contents
 
-        # Just make sure this call doesn't fail. Not sure what to check the
-        # return value against.
-        v.get_csig()
+    def test_get_content_with_children(self):
+        """Test calculating contents with child nodes
+        """
+        class DummyNode:
+            def __init__(self, csig):
+                self.csig = csig
+            def get_csig(self):
+                return self.csig
+
+        v = SCons.Node.Python.Value('aaa')
+        c1 = DummyNode(csig='csig1')
+        c2 = DummyNode(csig='csig2')
+
+        v.add_dependency([c1, c2])
+        contents = v.get_contents()
+        assert contents == v.get_text_contents().encode('utf-8'), contents
 
 
 class ValueNodeInfoTestCase(unittest.TestCase):