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/Action.py

@@ -1405,7 +1405,7 @@ def get_contents(self, target, source, env):
  except AttributeError:
  # No __call__() method, so it might be a builtin
  # or something like that. Do the best we can.
- contents = repr(actfunc)
+ contents = repr(actfunc).encode()
 
  return contents
 

SCons/ActionTests.py

@@ -2048,9 +2048,9 @@ def __call__(self):
  af = SCons.Action.ActionFactory(str, strfunc)
  ac = SCons.Action.ActionCaller(af, [], {})
  c = ac.get_contents([], [], Environment())
- assert c == "<built-in function str>" or \
- c == "<type 'str'>" or \
- c == "<class 'str'>", repr(c)
+ assert c == b"<built-in function str>" or \
+ c == b"<type 'str'>" or \
+ c == b"<class 'str'>", repr(c)
  # ^^ class str for python3
 
  def test___call__(self):

SCons/Executor.py

@@ -643,7 +643,7 @@ def get_action_side_effects(self):
  def __call__(self, *args, **kw):
  return 0
  def get_contents(self):
- return ''
+ return b''
  def _morph(self):
  """Morph this Null executor to a real Executor object."""
  batches = self.batches

SCons/Node/Alias.py

@@ -132,7 +132,7 @@ def get_contents(self):
  """The contents of an alias is the concatenation
  of the content signatures of all its sources."""
  childsigs = [n.get_csig() for n in self.children()]
- return ''.join(childsigs)
+ return ''.join(childsigs).encode('utf-8')
 
  def sconsign(self):
  """An Alias is not recorded in .sconsign files"""

SCons/Node/AliasTests.py

@@ -57,7 +57,7 @@ def __init__(self, contents):
  def get_csig(self):
  return self.contents
  def get_contents(self):
- return self.contents
+ return self.contents.encode()
 
  ans = SCons.Node.Alias.AliasNameSpace()
 
@@ -67,7 +67,7 @@ def get_contents(self):
  a.sources = [ DummyNode('one'), DummyNode('two'), DummyNode('three') ]
 
  c = a.get_contents()
- assert c == 'onetwothree', c
+ assert c == 'onetwothree'.encode('utf-8'), c
 
  def test_lookup(self):
  """Test the lookup() method

SCons/Node/FS.py

@@ -1856,7 +1856,8 @@ def scanner_key(self):
  def get_text_contents(self):
  """We already emit things in text, so just return the binary
  version."""
- return self.get_contents()
+ # Function get_contents_dir() returns utf-8 encoded text.
+ return self.get_contents().decode('utf-8')
 
  def get_contents(self):
  """Return content signatures and names of all our children

SCons/Node/FSTests.py

@@ -1411,7 +1411,7 @@ def nonexistent(method, s):
  # test Entry.get_contents()
  e = fs.Entry('does_not_exist')
  c = e.get_contents()
- assert c == "", c
+ assert c == b"", c
  assert e.__class__ == SCons.Node.FS.Entry
 
  test.write("file", "file\n")
@@ -1441,7 +1441,7 @@ def nonexistent(method, s):
  test.subdir("dir")
  e = fs.Entry('dir')
  c = e.get_contents()
- assert c == "", c
+ assert c == b"", c
  assert e.__class__ == SCons.Node.FS.Dir
 
  c = e.get_text_contents()
@@ -1455,7 +1455,7 @@ def nonexistent(method, s):
  e = fs.Entry('dangling_symlink')
  c = e.get_contents()
  assert e.__class__ == SCons.Node.FS.Entry, e.__class__
- assert c == "", c
+ assert c == b"", c
  c = e.get_text_contents()
  assert c == "", c
 
@@ -2049,14 +2049,14 @@ def test_get_contents(self):
  e = self.fs.Dir(os.path.join('d', 'empty'))
  s = self.fs.Dir(os.path.join('d', 'sub'))
 
- files = d.get_contents().split('\n')
+ files = d.get_contents().split('\n'.encode('utf-8'))
 
- assert e.get_contents() == '', e.get_contents()
+ assert e.get_contents() == b'', e.get_contents()
  assert e.get_text_contents() == '', e.get_text_contents()
- assert e.get_csig() + " empty" == files[0], files
- assert f.get_csig() + " f" == files[1], files
- assert g.get_csig() + " g" == files[2], files
- assert s.get_csig() + " sub" == files[3], files
+ assert (e.get_csig() + " empty").encode('utf-8') == files[0], files
+ assert (f.get_csig() + " f").encode('utf-8') == files[1], files
+ assert (g.get_csig() + " g").encode('utf-8') == files[2], files
+ assert (s.get_csig() + " sub").encode('utf-8') == files[3], files
 
  def test_implicit_re_scans(self):
  """Test that adding entries causes a directory to be re-scanned
@@ -2532,7 +2532,7 @@ def get_timestamp(self):
  return self.timestamp
 
  def get_contents(self):
- return self.name
+ return self.name.encode()
 
  def get_ninfo(self):
  """ mocked to ensure csig will equal the filename"""

SCons/Node/Python.py

@@ -138,26 +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
  """
- text_contents = self.get_text_contents()
- try:
- return text_contents.encode()
- except UnicodeDecodeError:
- # Already encoded as python2 str are bytes
- return text_contents
+ return self.get_text_contents().encode('utf-8')
 
  def changed_since_last_build(self, target, prev_ni):
  cur_csig = self.get_csig()
@@ -178,10 +172,9 @@ def get_csig(self, calc=None):
  except AttributeError:
  pass
 
- contents = self.get_text_contents()
+ 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,8 +103,58 @@ def test_get_csig(self):
  csig = v3.get_csig(None)
  assert csig == 'None', csig
 
+ def test_get_csig_with_children(self):
+ """Test calculating the content signature of a Value() object with child
+ nodes.
+ """
+ class DummyNode:
+ def __init__(self, csig):
+ self.csig = csig
+ def get_csig(self, calc=None):
+ return self.csig
+
+ v = SCons.Node.Python.Value('aaa')
+
+ c1 = DummyNode(csig='csig1')
+ c2 = DummyNode(csig='csig2')
+
+ v.add_dependency([c1, c2])
+ csig = v.get_csig(None)
+ assert csig == 'aaacsig1csig2', csig
+
+ def test_get_text_contents_with_children(self):
+ """Test calculating text 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])
+ text_contents = v.get_text_contents()
+ assert text_contents == 'aaacsig1csig2', text_contents
+
+ 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):
@@ -129,7 +179,7 @@ def test___init__(self):
  node._func_get_contents = 2 # Pretend to be a Dir.
  node.add_to_implicit([value])
  contents = node.get_contents()
- expected_contents = '%s %s\n' % (value.get_csig(), value.name)
+ expected_contents = ('%s %s\n' % (value.get_csig(), value.name)).encode('utf-8')
  assert contents == expected_contents
 
 

SCons/Node/__init__.py

@@ -206,7 +206,7 @@ def get_contents_entry(node):
  # string so calls to get_contents() in emitters and the
  # like (e.g. in qt.py) don't have to disambiguate by hand
  # or catch the exception.
- return ''
+ return b''
  else:
  return _get_contents_map[node._func_get_contents](node)
 
@@ -215,8 +215,8 @@ def get_contents_dir(node):
  separated by new-lines. Ensure that the nodes are sorted."""
  contents = []
  for n in sorted(node.children(), key=lambda t: t.name):
- contents.append('%s %s\n' % (n.get_csig(), n.name))
- return ''.join(contents)
+ contents.append(('%s %s\n' % (n.get_csig(), n.name)).encode('utf-8'))
+ return b''.join(contents)
 
 def get_contents_file(node):
  if not node.rexists():

SCons/SConfTests.py

@@ -160,7 +160,7 @@ def test_TryBuild(self):
 
  class MyAction:
  def get_contents(self, target, source, env):
- return 'MyBuilder-MyAction $SOURCE $TARGET'
+ return b'MyBuilder-MyAction $SOURCE $TARGET'
 
  class MyBuilder(SCons.Builder.BuilderBase):
  def __init__(self):

SCons/Scanner/ScannerTests.py

@@ -494,7 +494,7 @@ def rfile(self):
  def exists(self):
  return self._exists
  def get_contents(self):
- return self._contents
+ return self._contents.encode()
  def get_text_contents(self):
  return self._contents
  def get_dir(self):