Merge pull request #3814 from mwichmann/addmethod-clone

Fix/update global AddMethod

Author: bdbaddog

CHANGES.txt

@@ -65,6 +65,11 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Make sure cProfile is used if profiling - SCons was expecting
       the Util module to monkeypatch in cProfile as profile if available,
       but this is no longer being done.
+    - Cleanup in SCons.Util.AddMethod. If called with an environment instance
+      as the object to modify, the method would not be correctly set up in
+      any Clone of that instance.  Now tries to detect this and calls
+      MethodWrapper to set up the method the same way env.AddMethod does.
+      MethodWrapper moved to Util to avoid a circular import. Fixes #3028.
     - Some Python 2 compatibility code dropped
 
   From Simon Tegelid

SCons/Environment.py

@@ -54,6 +54,7 @@
 import SCons.Subst
 import SCons.Tool
 import SCons.Util
+from SCons.Util import MethodWrapper
 import SCons.Warnings
 
 class _Null:
@@ -180,48 +181,17 @@ def _delete_duplicates(l, keep_last):
 # Shannon at the following page (there called the "transplant" class):
 #
 # ASPN : Python Cookbook : Dynamically added methods to a class
-# http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/81732
+# https://code.activestate.com/recipes/81732/
 #
 # We had independently been using the idiom as BuilderWrapper, but
 # factoring out the common parts into this base class, and making
 # BuilderWrapper a subclass that overrides __call__() to enforce specific
 # Builder calling conventions, simplified some of our higher-layer code.
+#
+# Note: MethodWrapper moved to SCons.Util as it was needed there
+# and otherwise we had a circular import problem.
 
-class MethodWrapper:
-    """
-    A generic Wrapper class that associates a method (which can
-    actually be any callable) with an object.  As part of creating this
-    MethodWrapper object an attribute with the specified (by default,
-    the name of the supplied method) is added to the underlying object.
-    When that new "method" is called, our __call__() method adds the
-    object as the first argument, simulating the Python behavior of
-    supplying "self" on method calls.
-
-    We hang on to the name by which the method was added to the underlying
-    base class so that we can provide a method to "clone" ourselves onto
-    a new underlying object being copied (without which we wouldn't need
-    to save that info).
-    """
-    def __init__(self, object, method, name=None):
-        if name is None:
-            name = method.__name__
-        self.object = object
-        self.method = method
-        self.name = name
-        setattr(self.object, name, self)
-
-    def __call__(self, *args, **kwargs):
-        nargs = (self.object,) + args
-        return self.method(*nargs, **kwargs)
-
-    def clone(self, new_object):
-        """
-        Returns an object that re-binds the underlying "method" to
-        the specified new object.
-        """
-        return self.__class__(new_object, self.method, self.name)
-
-class BuilderWrapper(MethodWrapper):
+class BuilderWrapper(SCons.Util.MethodWrapper):
     """
     A MethodWrapper subclass that that associates an environment with
     a Builder.
@@ -248,7 +218,7 @@ def __call__(self, target=None, source=_null, *args, **kw):
             target = [target]
         if source is not None and not SCons.Util.is_List(source):
             source = [source]
-        return MethodWrapper.__call__(self, target, source, *args, **kw)
+        return super().__call__(target, source, *args, **kw)
 
     def __repr__(self):
         return '<BuilderWrapper %s>' % repr(self.name)
@@ -2377,7 +2347,7 @@ def __getattr__(self, name):
         # Environment they are being constructed with and so will not
         # have access to overrided values. So we rebuild them with the
         # OverrideEnvironment so they have access to overrided values.
-        if isinstance(attr, (MethodWrapper, BuilderWrapper)):
+        if isinstance(attr, MethodWrapper):
             return attr.clone(self)
         else:
             return attr

SCons/Environment.xml

@@ -286,54 +286,40 @@ until the Action object is actually used.
 </arguments>
 <summary>
 <para>
-When called with the
-<function>AddMethod</function>()
-form,
-adds the specified
-<parameter>function</parameter>
-to the specified
-<parameter>object</parameter>
-as the specified method
-<parameter>name</parameter>.
-When called using the
-&f-env-AddMethod; form,
-adds the specified
-<parameter>function</parameter>
-to the construction environment
-<replaceable>env</replaceable>
-as the specified method
-<parameter>name</parameter>.
-In both cases, if
-<parameter>name</parameter>
-is omitted or
-<constant>None</constant>,
-the name of the
-specified
-<parameter>function</parameter>
-itself is used for the method name.
+Adds <parameter>function</parameter> to an object as a method.
+<parameter>function</parameter> will be called with an instance
+object as the first argument as for other methods.
+If <parameter>name</parameter> is given, it is used as
+the name of the new method, else the name of
+<parameter>function</parameter> is used.
+</para>
+<para>
+When the global function &f-AddMethod; is called,
+the object to add the method to must be passed as the first argument;
+typically this will be &Environment;,
+in order to create a method which applies to all &consenvs;
+subsequently constructed.
+When called using the &f-env-AddMethod; form,
+the method is added to the specified &consenv; only.
+Added methods propagate through &f-env-Clone; calls.
 </para>
 
 <para>
 Examples:
 </para>
 
 <example_commands>
-# Note that the first argument to the function to
-# be attached as a method must be the object through
-# which the method will be called; the Python
-# convention is to call it 'self'.
+# Function to add must accept an instance argument.
+# The Python convention is to call this 'self'.
 def my_method(self, arg):
     print("my_method() got", arg)
 
-# Use the global AddMethod() function to add a method
-# to the Environment class.  This
+# Use the global function to add a method to the Environment class:
 AddMethod(Environment, my_method)
 env = Environment()
 env.my_method('arg')
 
-# Add the function as a method, using the function
-# name for the method call.
-env = Environment()
+# Use the optional name argument to set the name of the method:
 env.AddMethod(my_method, 'other_method_name')
 env.other_method_name('another arg')
 </example_commands>

SCons/EnvironmentTests.py

@@ -33,7 +33,13 @@
 import TestCmd
 import TestUnit
 
-from SCons.Environment import *
+from SCons.Environment import (
+    Environment,
+    NoSubstitutionProxy,
+    OverrideEnvironment,
+    SubstitutionEnvironment,
+    is_valid_construction_var,
+)
 import SCons.Warnings
 
 def diff_env(env1, env2):
@@ -721,7 +727,7 @@ def func2(self, arg=''):
         r = env4.func2()
         assert r == 'func2-4', r
 
-        # Test that clones don't re-bind an attribute that the user
+        # Test that clones don't re-bind an attribute that the user set.
         env1 = Environment(FOO = '1')
         env1.AddMethod(func2)
         def replace_func2():
@@ -731,6 +737,18 @@ def replace_func2():
         r = env2.func2()
         assert r == 'replace_func2', r
 
+        # Test clone rebinding if using global AddMethod.
+        env1 = Environment(FOO='1')
+        SCons.Util.AddMethod(env1, func2)
+        r = env1.func2()
+        assert r == 'func2-1', r
+        r = env1.func2('-xxx')
+        assert r == 'func2-1-xxx', r
+        env2 = env1.Clone(FOO='2')
+        r = env2.func2()
+        assert r == 'func2-2', r
+
+
     def test_Override(self):
         """Test overriding construction variables"""
         env = SubstitutionEnvironment(ONE=1, TWO=2, THREE=3, FOUR=4)