Merge pull request #3895 from bdbaddog/allow_subst_callables_to_handle_default_value_args

Allow subst callables to handle default value args and functions constructed using functools.partial to set extraneous args

4 files changed, 81 insertions, 9 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 3384bb7..72a32d5 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -8,6 +8,12 @@ NOTE: The 4.0.0 Release of SCons dropped Python 2.7 Support
 
 RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
+  From William Deegan:
+    - Improve Subst()'s logic to check for proper callable function or class's argument list.
+      It will now allow callables with expected args, and any extra args as long as they
+      have default arguments. Additionally functions with no defaults for extra arguments
+      as long as they are set using functools.partial to create a new callable which set them.
+
   From Daniel Moody:
     - Update CacheDir to use uuid for tmpfile uniqueness instead of pid.
       This fixes cases for shared cache where two systems write to the same
diff --git a/RELEASE.txt b/RELEASE.txt
index 89d634c..f794a82 100755
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -19,8 +19,10 @@ DEPRECATED FUNCTIONALITY
 CHANGED/ENHANCED EXISTING FUNCTIONALITY
 ---------------------------------------
 
-- List modifications to existing features, where the previous behavior
-  wouldn't actually be considered a bug
+    - Improve Subst()'s logic to check for proper callable function or class's argument list.
+      It will now allow callables with expected args, and any extra args as long as they
+      have default arguments. Additionally functions with no defaults for extra arguments
+      as long as they are set using functools.partial to create a new callable which set them.
 
 FIXES
 -----
diff --git a/SCons/Subst.py b/SCons/Subst.py
index ea756ff..f23b2d0 100644
--- a/SCons/Subst.py
+++ b/SCons/Subst.py
@@ -25,9 +25,9 @@
 
 import collections
 import re
-from inspect import signature
-import SCons.Errors
+from inspect import signature, Parameter
 
+import SCons.Errors
 from SCons.Util import is_String, is_Sequence
 
 # Indexed by the SUBST_* constants below.
@@ -328,6 +328,8 @@ def subst_dict(target, source):
     return dict
 
 
+_callable_args_set = {'target', 'source', 'env', 'for_signature'}
+
 class StringSubber:
     """A class to construct the results of a scons_subst() call.
 
@@ -335,6 +337,8 @@ class StringSubber:
     source with two methods (substitute() and expand()) that handle
     the expansion.
     """
+
+
     def __init__(self, env, mode, conv, gvars):
         self.env = env
         self.mode = mode
@@ -419,8 +423,11 @@ class StringSubber:
             # SCons has the unusual Null class where any __getattr__ call returns it's self, 
             # which does not work the signature module, and the Null class returns an empty
             # string if called on, so we make an exception in this condition for Null class
-            if (isinstance(s, SCons.Util.Null) or
-                set(signature(s).parameters.keys()) == set(['target', 'source', 'env', 'for_signature'])):
+            # Also allow callables where the only non default valued args match the expected defaults
+            # this should also allow functools.partial's to work.
+            if isinstance(s, SCons.Util.Null) or {k for k, v in signature(s).parameters.items() if
+                                                  k in _callable_args_set or v.default == Parameter.empty} == _callable_args_set:
+
                 s = s(target=lvars['TARGETS'],
                      source=lvars['SOURCES'],
                      env=self.env,
@@ -593,8 +600,11 @@ class ListSubber(collections.UserList):
             # SCons has the unusual Null class where any __getattr__ call returns it's self, 
             # which does not work the signature module, and the Null class returns an empty
             # string if called on, so we make an exception in this condition for Null class
-            if (isinstance(s, SCons.Util.Null) or
-                set(signature(s).parameters.keys()) == set(['target', 'source', 'env', 'for_signature'])):
+            # Also allow callables where the only non default valued args match the expected defaults
+            # this should also allow functools.partial's to work.
+            if isinstance(s, SCons.Util.Null) or {k for k, v in signature(s).parameters.items() if
+                                                  k in _callable_args_set or v.default == Parameter.empty} == _callable_args_set:
+
                 s = s(target=lvars['TARGETS'],
                      source=lvars['SOURCES'],
                      env=self.env,
diff --git a/SCons/SubstTests.py b/SCons/SubstTests.py
index bdeb01e..b2951fb 100644
--- a/SCons/SubstTests.py
+++ b/SCons/SubstTests.py
@@ -25,6 +25,7 @@ import SCons.compat
 
 import os
 import unittest
+from functools import partial
 
 
 import SCons.Errors
@@ -92,6 +93,23 @@ class CmdGen2:
         assert for_signature == self.expect_for_signature, for_signature
         return [ self.mystr, env.Dictionary('BAR') ]
 
+
+def CallableWithDefault(target, source, env, for_signature, other_value="default"):
+    assert str(target) == 't', target
+    assert str(source) == 's', source
+    return "CallableWithDefault: %s"%other_value
+
+PartialCallable = partial(CallableWithDefault, other_value="partial")
+
+def CallableWithNoDefault(target, source, env, for_signature, other_value):
+    assert str(target) == 't', target
+    assert str(source) == 's', source
+    return "CallableWithNoDefault: %s"%other_value
+
+PartialCallableNoDefault = partial(CallableWithNoDefault, other_value="partialNoDefault")
+
+
+
 if os.sep == '/':
     def cvt(str):
         return str
@@ -190,6 +208,10 @@ class SubstTestCase(unittest.TestCase):
         'CMDGEN1'   : CmdGen1,
         'CMDGEN2'   : CmdGen2,
 
+        'CallableWithDefault': CallableWithDefault,
+        'PartialCallable' : PartialCallable,
+        'PartialCallableNoDefault' : PartialCallableNoDefault,
+
         'LITERALS'  : [ Literal('foo\nwith\nnewlines'),
                         Literal('bar\nwith\nnewlines') ],
 
@@ -238,6 +260,7 @@ class SubstTestCase(unittest.TestCase):
                   'gvars' : env.Dictionary()}
 
         failed = 0
+        case_count = 0
         while cases:
             input, expect = cases[:2]
             expect = convert(expect)
@@ -250,12 +273,14 @@ class SubstTestCase(unittest.TestCase):
             else:
                 if result != expect:
                     if failed == 0: print()
-                    print("    input %s => \n%s did not match \n%s" % (repr(input), repr(result), repr(expect)))
+                    print("[%4d]    input %s => \n%s did not match \n%s" % (case_count, repr(input), repr(result), repr(expect)))
                     failed = failed + 1
             del cases[:2]
+            case_count += 1
         fmt = "%d %s() cases failed"
         assert failed == 0, fmt % (failed, function.__name__)
 
+
 class scons_subst_TestCase(SubstTestCase):
 
     # Basic tests of substitution functionality.
@@ -518,6 +543,35 @@ class scons_subst_TestCase(SubstTestCase):
                              gvars=gvars)
         assert newcom == "test foo bar with spaces.out s t", newcom
 
+    def test_subst_callable_with_default_expansion(self):
+        """Test scons_subst():  expanding a callable with a default value arg"""
+        env = DummyEnv(self.loc)
+        gvars = env.Dictionary()
+        newcom = scons_subst("test $CallableWithDefault $SOURCES $TARGETS", env,
+                             target=self.MyNode('t'), source=self.MyNode('s'),
+                             gvars=gvars)
+        assert newcom == "test CallableWithDefault: default s t", newcom
+
+    def test_subst_partial_callable_with_default_expansion(self):
+        """Test scons_subst():  expanding a functools.partial callable which sets
+           the default value in the callable"""
+        env = DummyEnv(self.loc)
+        gvars = env.Dictionary()
+        newcom = scons_subst("test $PartialCallable $SOURCES $TARGETS", env,
+                             target=self.MyNode('t'), source=self.MyNode('s'),
+                             gvars=gvars)
+        assert newcom == "test CallableWithDefault: partial s t", newcom
+
+    def test_subst_partial_callable_with_no_default_expansion(self):
+        """Test scons_subst():  expanding a functools.partial callable which sets
+           the value for extraneous function argument"""
+        env = DummyEnv(self.loc)
+        gvars = env.Dictionary()
+        newcom = scons_subst("test $PartialCallableNoDefault $SOURCES $TARGETS", env,
+                             target=self.MyNode('t'), source=self.MyNode('s'),
+                             gvars=gvars)
+        assert newcom == "test CallableWithNoDefault: partialNoDefault s t", newcom
+
     def test_subst_attribute_errors(self):
         """Test scons_subst():  handling attribute errors"""
         env = DummyEnv(self.loc)