Merge pull request #3213 from bdbaddog/fix_3212_TryCompile_With_Py3_CacheDir

Fix Issue # 3212 try compile with py3 cache dir

7 files changed, 104 insertions, 2 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index c8ec110..9b3eec4 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -48,6 +48,8 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE
     - Add alternate path to QT install for Centos in qt tool: /usr/lib64/qt-3.3/bin
     - Fix GH Issue #2580 - # in FRAMEWORKPATH doesn't get properly expanded. The # is left in the
       command line.
+    - Fix GH Issue #3212 - Use of Py3 and CacheDir + Configure's TryCompile (or likely and Python Value Nodes)
+      yielded trying to combine strings and bytes which threw exception.
 
   From Andrew Featherstone
     - Removed unused --warn options from the man page and source code.
diff --git a/src/engine/SCons/CacheDir.py b/src/engine/SCons/CacheDir.py
index ab80808..ab23f31 100644
--- a/src/engine/SCons/CacheDir.py
+++ b/src/engine/SCons/CacheDir.py
@@ -221,7 +221,9 @@ class CacheDir(object):
             return None, None
 
         sig = node.get_cachedir_bsig()
+
         subdir = sig[:self.config['prefix_len']].upper()
+
         dir = os.path.join(self.path, subdir)
         return dir, os.path.join(dir, sig)
 
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index bce1549..01d01cd 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -450,6 +450,8 @@ class Executor(object, with_metaclass(NoSlotsPyPy)):
         """Fetch the signature contents.  This is the main reason this
         class exists, so we can compute this once and cache it regardless
         of how many target or source Nodes there are.
+
+        Returns bytes
         """
         try:
             return self._memo['get_contents']
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index bc15064..5dceaa0 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -3382,6 +3382,8 @@ class File(Base):
         because multiple targets built by the same action will all
         have the same build signature, and we have to differentiate
         them somehow.
+
+        Signature should normally be string of hex digits.
         """
         try:
             return self.cachesig
@@ -3391,10 +3393,13 @@ class File(Base):
         # Collect signatures for all children
         children = self.children()
         sigs = [n.get_cachedir_csig() for n in children]
+
         # Append this node's signature...
         sigs.append(self.get_contents_sig())
+
         # ...and it's path
         sigs.append(self.get_internal_path())
+
         # Merge this all into a single signature
         result = self.cachesig = SCons.Util.MD5collect(sigs)
         return result
diff --git a/src/engine/SCons/Node/Python.py b/src/engine/SCons/Node/Python.py
index 8c47c97..4a62f04 100644
--- a/src/engine/SCons/Node/Python.py
+++ b/src/engine/SCons/Node/Python.py
@@ -137,6 +137,10 @@ class Value(SCons.Node.Node):
         return contents
 
     def get_contents(self):
+        """
+        Get contents for signature calculations.
+        :return: bytes
+        """
         text_contents = self.get_text_contents()
         try:
             return text_contents.encode()
@@ -155,12 +159,17 @@ class Value(SCons.Node.Node):
     def get_csig(self, calc=None):
         """Because we're a Python value node and don't have a real
         timestamp, we get to ignore the calculator and just use the
-        value contents."""
+        value contents.
+
+        Returns string. Ideally string of hex digits. (Not bytes)
+        """
         try:
             return self.ninfo.csig
         except AttributeError:
             pass
-        contents = self.get_contents()
+
+        contents = self.get_text_contents()
+
         self.get_ninfo().csig = contents
         return contents
 
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 424c694..b568ce5 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -1459,6 +1459,11 @@ else:
         md5 = True
 
         def MD5signature(s):
+            """
+            Generate a String of Hex digits representing the md5 signature of the string
+            :param s: either string or bytes. Normally should be bytes
+            :return: String of hex digits
+            """
             m = hashlib.md5()
 
             try:
@@ -1469,6 +1474,11 @@ else:
             return m.hexdigest()
 
         def MD5filesignature(fname, chunksize=65536):
+            """
+            :param fname:
+            :param chunksize:
+            :return: String of Hex digits
+            """
             m = hashlib.md5()
             f = open(fname, "rb")
             while True:
@@ -1479,6 +1489,7 @@ else:
             f.close()
             return m.hexdigest()
 
+
 def MD5collect(signatures):
     """
     Collects a list of signatures into an aggregate signature.
diff --git a/test/CacheDir/CacheDir_TryCompile.py b/test/CacheDir/CacheDir_TryCompile.py
new file mode 100644
index 0000000..bb22835
--- /dev/null
+++ b/test/CacheDir/CacheDir_TryCompile.py
@@ -0,0 +1,71 @@
+#!/usr/bin/env python
+#
+# __COPYRIGHT__
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# 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 that CacheDir functions with TryCompile.
+
+With Py3 there was an issue where the generated cache signature from Python Value nodes
+could be bytes instead of a string which would fail when combining cache signatures
+which ended up a mixture of bytes and strings.
+"""
+
+import os
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+cache = test.workpath('cache')
+
+test.subdir('cache', 'src')
+
+test.write(['src', 'SConstruct'], """\
+DefaultEnvironment(tools=[])
+env = Environment()
+env.CacheDir(r'%(cache)s')
+
+conf = Configure(env)
+
+conf.TryCompile('int a;', '.c')
+
+env = conf.Finish()
+""" % locals())
+
+# Verify that a normal build works correctly, and clean up.
+# This should populate the cache with our derived files.
+test.run(chdir = 'src', arguments = '.')
+
+test.up_to_date(chdir = 'src', arguments = '.')
+
+test.run(chdir = 'src', arguments = '-c .')
+
+test.pass_test()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4: