diff --git a/CHANGES.txt b/CHANGES.txt index 6ca145e58d..18ef3a3905 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -25,6 +25,15 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Fix minor bug affecting SCons.Node.FS.File.get_csig()'s usage of the MD5 chunksize. User-facing behavior does not change with this fix (GH Issue #3726). + 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. + From Joachim Kuebart: - Suppress missing SConscript deprecation warning if `must_exist=False` is used. diff --git a/SCons/Action.py b/SCons/Action.py index 1f499eee5a..abe7172124 100644 --- a/SCons/Action.py +++ b/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 diff --git a/SCons/ActionTests.py b/SCons/ActionTests.py index 81dacfaf15..09fa118262 100644 --- a/SCons/ActionTests.py +++ b/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 == "" or \ - c == "" or \ - c == "", repr(c) + assert c == b"" or \ + c == b"" or \ + c == b"", repr(c) # ^^ class str for python3 def test___call__(self): diff --git a/SCons/Executor.py b/SCons/Executor.py index f6582298c6..c317e230c4 100644 --- a/SCons/Executor.py +++ b/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 diff --git a/SCons/Node/Alias.py b/SCons/Node/Alias.py index 55d94f9808..64186106e4 100644 --- a/SCons/Node/Alias.py +++ b/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""" diff --git a/SCons/Node/AliasTests.py b/SCons/Node/AliasTests.py index 701cdea680..cd88ab40c2 100644 --- a/SCons/Node/AliasTests.py +++ b/SCons/Node/AliasTests.py @@ -56,7 +56,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() @@ -66,7 +66,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 diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py index ad9dd6f0e7..98f972c95d 100644 --- a/SCons/Node/FS.py +++ b/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 diff --git a/SCons/Node/FSTests.py b/SCons/Node/FSTests.py index e17ca3d4da..c3f1c393f6 100644 --- a/SCons/Node/FSTests.py +++ b/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 @@ -2048,14 +2048,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_md5_chunksize(self): """ @@ -2590,7 +2590,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""" diff --git a/SCons/Node/Python.py b/SCons/Node/Python.py index 68c6ee88be..63ddb8496c 100644 --- a/SCons/Node/Python.py +++ b/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): diff --git a/SCons/Node/PythonTests.py b/SCons/Node/PythonTests.py index b6fa859d2d..2acd747926 100644 --- a/SCons/Node/PythonTests.py +++ b/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 diff --git a/SCons/Node/__init__.py b/SCons/Node/__init__.py index 28772ebf15..b252ace41d 100644 --- a/SCons/Node/__init__.py +++ b/SCons/Node/__init__.py @@ -205,7 +205,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) @@ -214,8 +214,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(): diff --git a/SCons/SConfTests.py b/SCons/SConfTests.py index 7faabdde19..772958c832 100644 --- a/SCons/SConfTests.py +++ b/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 Attrs: __slots__ = ('shared', '__dict__') diff --git a/SCons/Scanner/ScannerTests.py b/SCons/Scanner/ScannerTests.py index 34709aefc6..f772ecb4cd 100644 --- a/SCons/Scanner/ScannerTests.py +++ b/SCons/Scanner/ScannerTests.py @@ -493,7 +493,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):