PY2/3 Fix action object content signiture to be stable. Note need expected value for each change in python bytecode. Currently that means one for each: py2.7, py3.5, py3.6

2 files changed, 75 insertions, 25 deletions

diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 118c495..bf611af 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -105,6 +105,8 @@ import pickle
 import re
 import sys
 import subprocess
+import itertools
+import inspect
 
 import SCons.Debug
 from SCons.Debug import logInstanceCreation
@@ -195,8 +197,11 @@ def _object_contents(obj):
                 except AttributeError as ae:
                     # Should be a pickle-able Python object.
                     try:
-                        return pickle.dumps(obj, ACTION_SIGNATURE_PICKLE_PROTOCOL)
-                    except (pickle.PicklingError, TypeError, AttributeError):
+                        return _object_instance_content(obj)
+                        # pickling an Action instance or object doesn't yield a stable
+                        # content as instance property may be dumped in different orders
+                        # return pickle.dumps(obj, ACTION_SIGNATURE_PICKLE_PROTOCOL)
+                    except (pickle.PicklingError, TypeError, AttributeError) as ex:
                         # This is weird, but it seems that nested classes
                         # are unpickable. The Python docs say it should
                         # always be a PicklingError, but some Python
@@ -308,15 +313,12 @@ def _function_contents(func):
         defaults.extend(b')')
 
         contents.append(defaults)
-
-        # contents.append(bytearray(',(','utf-8') + b','.join(function_defaults_contents) + bytearray(')','utf-8'))
     else:
         contents.append(b',()')
 
     # The function contents depends on the closure captured cell values.
     closure = func.__closure__ or []
 
-    #xxx = [_object_contents(x.cell_contents) for x in closure]
     try:
         closure_contents = [_object_contents(x.cell_contents) for x in closure]
     except AttributeError:
@@ -325,10 +327,61 @@ def _function_contents(func):
     contents.append(b',(')
     contents.append(bytearray(b',').join(closure_contents))
     contents.append(b')')
-    # contents.append(b'BBBBBBBB')
 
-    return bytearray(b'').join(contents)
+    retval = bytearray(b'').join(contents)
+
+    # print("ReTVAL:%s"%retval)
+    return retval
+
 
+def _object_instance_content(obj):
+    """
+    Returns consistant content for a action class or an instance thereof
+    :param obj: Should be either and action class or an instance thereof
+    :return: bytearray or bytes representing the obj suitable for generating
+             a signiture from.
+    """
+    retval = bytearray()
+
+
+    inst_class = obj.__class__
+    inst_class_name = bytearray(obj.__class__.__name__,'utf-8')
+    inst_class_module = bytearray(obj.__class__.__module__,'utf-8')
+    inst_class_hierarchy = bytearray(repr(inspect.getclasstree([obj.__class__,])),'utf-8')
+    # print("ICH:%s : %s"%(inst_class_hierarchy, repr(obj)))
+
+    properties = [(p, getattr(obj, p, "None")) for p in dir(obj) if p[:2] != '__' and not inspect.ismethod(getattr(obj, p))]
+    properties.sort()
+    properties_str = ','.join(["%s=%s"%(p[0],p[1]) for p in properties])
+    properties_bytes = bytearray(properties_str,'utf-8')
+
+    methods = [p for p in dir(obj) if inspect.ismethod(getattr(obj, p))]
+    methods.sort()
+
+    method_contents = []
+    for m in methods:
+        # print("Method:%s"%m)
+        v = _function_contents(getattr(obj, m))
+        # print("[%s->]V:%s [%s]"%(m,v,type(v)))
+        method_contents.append(v)
+
+    retval = bytearray(b'{')
+    retval.extend(inst_class_name)
+    retval.extend(b":")
+    retval.extend(inst_class_module)
+    retval.extend(b'}[[')
+    retval.extend(inst_class_hierarchy)
+    retval.extend(b']]{{')
+    retval.extend(bytearray(b",").join(method_contents))
+    retval.extend(b"}}")
+    return retval
+
+    # print("class          :%s"%inst_class)
+    # print("class_name     :%s"%inst_class_name)
+    # print("class_module   :%s"%inst_class_module)
+    # print("Class hier     :\n%s"%pp.pformat(inst_class_hierarchy))
+    # print("Inst Properties:\n%s"%pp.pformat(properties))
+    # print("Inst Methods   :\n%s"%pp.pformat(methods))
 
 def _actionAppend(act1, act2):
     # This function knows how to slap two actions together.
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 34d9ffc..7980e39 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -98,7 +98,9 @@ scons_env = SCons.Environment.Environment()
 
 # Capture all the stuff the Actions will print,
 # so it doesn't clutter the output.
-sys.stdout = io.StringIO()
+
+# TODO.. uncomment
+# sys.stdout = io.StringIO()
 
 class CmdStringHolder(object):
     def __init__(self, cmd, literal=None):
@@ -2054,7 +2056,7 @@ class ActionCompareTestCase(unittest.TestCase):
         assert dog.get_name(env) == 'DOG', dog.get_name(env)
 
 
-class TestClass:
+class TestClass(object):
     """A test class used by ObjectContentsTestCase.test_object_contents"""
     def __init__(self):
         self.a = "a"
@@ -2081,7 +2083,6 @@ class ObjectContentsTestCase(unittest.TestCase):
         assert c in expected, "Got\n"+repr(c)+"\nExpected one of \n"+"\n".join([repr(e) for e in expected])
 
 
-    # @unittest.skip("Results vary between py2 and py3, not sure if test makes sense to implement")
     def test_object_contents(self):
         """Test that Action._object_contents works"""
 
@@ -2089,20 +2090,18 @@ class ObjectContentsTestCase(unittest.TestCase):
         o = TestClass()
         c = SCons.Action._object_contents(o)
 
+        # c = SCons.Action._object_instance_content(o)
+
+        # Since the python bytecode has per version differences, we need different expected results per version
         if TestCmd.IS_PY3:
-            if TestCmd.IS_WINDOWS:
-                expected = [b'ccopy_reg\n_reconstructor\nq\x00(c__main__\nTestClass\nq\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04}q\x05(X\x01\x00\x00\x00aq\x06h\x06X\x01\x00\x00\x00bq\x07h\x07ub.', # py 3.6
-                b'ccopy_reg\n_reconstructor\nq\x00(c__main__\nTestClass\nq\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04}q\x05(X\x01\x00\x00\x00bq\x06h\x06X\x01\x00\x00\x00aq\x07h\x07ub.', # py 3.5
-                ]
-            else:
-                expected = [b'ccopy_reg\n_reconstructor\nq\x00(c__main__\nTestClass\nq\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04}q\x05(X\x01\x00\x00\x00bq\x06h\x06X\x01\x00\x00\x00aq\x07h\x07ub.']
+            if sys.version_info[:2] == (3,5):
+                expected = bytearray(b"{TestClass:__main__}[[[(<class \'object\'>, ()), [(<class \'__main__.TestClass\'>, (<class \'object\'>,))]]]]{{1, 1, 0, 0,({str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}},{str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}}),({str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}},{str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}}),(d\x01\x00|\x00\x00_\x00\x00d\x02\x00|\x00\x00_\x01\x00d\x00\x00S),(),(),2, 2, 0, 0,(),(),(d\x00\x00S),(),()}}")
+            elif sys.version_info[:2] == (3,6):
+                expected = bytearray(b"{TestClass:__main__}[[[(<class \'object\'>, ()), [(<class \'__main__.TestClass\'>, (<class \'object\'>,))]]]]{{1, 1, 0, 0,({str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}},{str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}}),({str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}},{str:builtins}[[[(<class \'object\'>, ()), [(<class \'str\'>, (<class \'object\'>,))]]]]{{}}),(d\x01|\x00_\x00d\x02|\x00_\x01d\x00S\x00),(),(),2, 2, 0, 0,(),(),(d\x00S\x00),(),()}}")
         else:
-            if TestCmd.IS_WINDOWS:
-                expected = [b'(c__main__\nTestClass\nq\x01oq\x02}q\x03(U\x01aU\x01aU\x01bU\x01bub.']
-            else:
-                expected = [b'(c__main__\nTestClass\nq\x01oq\x02}q\x03(U\x01aU\x01aU\x01bU\x01bub.']
+            expected = bytearray(b"{TestClass:__main__}[[[(<type \'object\'>, ()), [(<class \'__main__.TestClass\'>, (<type \'object\'>,))]]]]{{1, 1, 0, 0,({str:__builtin__}[[[(<type \'basestring\'>, (<type \'object\'>,)), [(<type \'str\'>, (<type \'basestring\'>,))]]]]{{}},{str:__builtin__}[[[(<type \'basestring\'>, (<type \'object\'>,)), [(<type \'str\'>, (<type \'basestring\'>,))]]]]{{}}),({str:__builtin__}[[[(<type \'basestring\'>, (<type \'object\'>,)), [(<type \'str\'>, (<type \'basestring\'>,))]]]]{{}},{str:__builtin__}[[[(<type \'basestring\'>, (<type \'object\'>,)), [(<type \'str\'>, (<type \'basestring\'>,))]]]]{{}}),(d\x01\x00|\x00\x00_\x00\x00d\x02\x00|\x00\x00_\x01\x00d\x00\x00S),(),(),2, 2, 0, 0,(),(),(d\x00\x00S),(),()}}")
 
-        assert c in expected, "Got\n"+repr(c)+"\nExpected one of \n"+"\n".join([repr(e) for e in expected])
+        assert c == expected, "Got\n"+repr(c)+"\nExpected \n"+"\n"+repr(expected)
 
     # @unittest.skip("Results vary between py2 and py3, not sure if test makes sense to implement")
     def test_code_contents(self):
@@ -2122,9 +2121,6 @@ class ObjectContentsTestCase(unittest.TestCase):
         assert c in expected, "Got\n"+repr(c)+"\nExpected one of \n"+"\n".join([repr(e) for e in expected])
 
 
-
-
-
 if __name__ == "__main__":
     suite = unittest.TestSuite()
     tclasses = [ _ActionActionTestCase,
@@ -2142,7 +2138,8 @@ if __name__ == "__main__":
         names = unittest.getTestCaseNames(tclass, 'test_')
         suite.addTests(list(map(tclass, names)))
 
-    TestUnit.run(suite)
+    # TestUnit.run(suite)
+    unittest.main()
 
 # Local Variables:
 # tab-width:4