Deprecate the overrides Builder() keyword argument in favor of specifying values directly as keyword arguments, like we do for other functions and methods.

8 files changed, 64 insertions, 14 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 2f1540b..18181fc 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -154,6 +154,13 @@ RELEASE 0.95 - XXX
     --debug=count and --debug=objects only print anything when run
     under Python 2.1 or later.
 
+  - Deprecate the "overrides" keyword argument to Builder() creation
+    in favor of using keyword argument values directly (like we do
+    for builder execution and the like).
+
+  - Always use the Builder overrides in substitutions, not just if
+    there isn't a target-specific environment.
+
   From Vincent Risi:
 
   - Add support for the bcc32, ilink32 and tlib Borland tools.
diff --git a/src/RELEASE.txt b/src/RELEASE.txt
index b52d3ad..54a079b 100644
--- a/src/RELEASE.txt
+++ b/src/RELEASE.txt
@@ -43,6 +43,19 @@ RELEASE 0.95 - XXX
       into the SCons internals and calling either of the SCons.Util
       functions directly.)
 
+    - The "overrides" keyword argument to calls to the Builder() function
+      used to create new Builder objects has been deprecated and will
+      be removed in a future release.  The items in an "overrides"
+      dictionary should now be specified as keyword arguments:
+
+          # Old way, "overrides" is a dictionary:
+          Builder(action = 'cp $TDIR/$TARGET $SDIR/$SOURCE',
+                  overrides = {'TDIR' : dir1, 'SDIR' : dir2})
+
+          # New way, items are specified as keyword arguments:
+          Builder(action = 'cp $TDIR/$TARGET $SDIR/$SOURCE',
+                  TDIR = dir1, SDIR = dir2)
+
   SCons is developed with an extensive regression test suite, and a
   rigorous development methodology for continually improving that suite.
   Because of this, SCons is of sufficient quality that you can use it
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 3010e98..6714a3d 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -269,7 +269,7 @@ class BuilderBase:
                         emitter = None,
                         multi = 0,
                         env = None,
-                        overrides = {}):
+                        **overrides):
         if __debug__: logInstanceCreation(self, 'BuilderBase')
         self.action = SCons.Action.Action(action)
         self.multi = multi
@@ -280,6 +280,12 @@ class BuilderBase:
             suffix = CallableSelector(suffix)
         self.suffix = suffix
         self.env = env
+        if overrides.has_key('overrides'):
+            SCons.Warnings.warn(SCons.Warnings.DeprecatedWarning,
+                "The \"overrides\" keyword to Builder() creation has been deprecated;\n" +\
+                "\tspecify the items as keyword arguments to the Builder() call instead.")
+            overrides.update(overrides['overrides'])
+            del overrides['overrides']
         self.overrides = overrides
 
         self.set_src_suffix(src_suffix)
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index cf11f6b..b03a99e 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -160,6 +160,15 @@ class MyNode(MyNode_without_target_from_source):
 
 class BuilderTestCase(unittest.TestCase):
 
+    def test__init__(self):
+        """Test simple Builder creation
+        """
+        builder = SCons.Builder.Builder(action="foo")
+        assert not builder is None, builder
+        builder = SCons.Builder.Builder(action="foo", OVERRIDE='x')
+        x = builder.overrides['OVERRIDE']
+        assert x == 'x', x
+
     def test__nonzero__(self):
         """Test a builder raising an exception when __nonzero__ is called
         """
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index 797bdda..2a0882e 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -64,12 +64,13 @@ class Executor:
                 # So use the environment associated with the Builder
                 # itself.
                 env = self.builder.env
-                overrides = self.builder.overrides
             else:
                 # The normal case:  use the Environment that was
                 # used to specify how these targets will be built.
                 env = self.env
-                overrides = self.overrides
+            overrides = {}
+            overrides.update(self.builder.overrides)
+            overrides.update(self.overrides)
             self.build_env = env.Override(overrides)
             return self.build_env
 
diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py
index 6ead306..2a79e2d 100644
--- a/src/engine/SCons/ExecutorTests.py
+++ b/src/engine/SCons/ExecutorTests.py
@@ -75,24 +75,38 @@ class ExecutorTestCase(unittest.TestCase):
 
     def test_get_build_env(self):
         """Test fetching and generating a build environment"""
-        x = SCons.Executor.Executor('b', 'e', 'o', 't', ['s1', 's2'])
+        x = SCons.Executor.Executor(MyBuilder('e', {}),
+                                    'e',
+                                    {},
+                                    't',
+                                    ['s1', 's2'])
         x.build_env = 'eee'
         be = x.get_build_env()
         assert be == 'eee', be
 
-        x = SCons.Executor.Executor('b',
+        x = SCons.Executor.Executor(MyBuilder('e', {}),
                                     MyEnvironment(X='xxx'),
-                                    {'O':'ooo'},
+                                    {'O':'o2'},
                                     't',
                                     ['s1', 's2'])
         be = x.get_build_env()
-        assert be == {'O':'ooo', 'X':'xxx'}, be
+        assert be == {'O':'o2', 'X':'xxx'}, be
 
         env = MyEnvironment(Y='yyy')
-        over = {'O':'ooo'}
-        x = SCons.Executor.Executor(MyBuilder(env, over), None, {}, 't', 's')
+        x = SCons.Executor.Executor(MyBuilder(env, {'O':'ob3'}),
+                                    None,
+                                    {'O':'oo3'},
+                                    't',
+                                    's')
+        be = x.get_build_env()
+        assert be == {'O':'oo3', 'Y':'yyy'}, be
+        x = SCons.Executor.Executor(MyBuilder(env, {'O':'ob3'}),
+                                    None,
+                                    {},
+                                    't',
+                                    's')
         be = x.get_build_env()
-        assert be == {'O':'ooo', 'Y':'yyy'}, be
+        assert be == {'O':'ob3', 'Y':'yyy'}, be
 
     def test_get_action_list(self):
         """Test fetching and generating an action list"""
diff --git a/src/engine/SCons/Tool/CVS.py b/src/engine/SCons/Tool/CVS.py
index b3bd166..6d5b046 100644
--- a/src/engine/SCons/Tool/CVS.py
+++ b/src/engine/SCons/Tool/CVS.py
@@ -51,8 +51,8 @@ def generate(env):
            env['CVSCOM']   = '$CVS $CVSFLAGS co $CVSCOFLAGS -d ${TARGET.dir} $CVSMODULE${TARGET.posix}'
         return SCons.Builder.Builder(action = '$CVSCOM',
                                      env = env,
-                                     overrides = {'CVSREPOSITORY':repos,
-                                                  'CVSMODULE':module})
+                                     CVSREPOSITORY = repos,
+                                     CVSMODULE = module)
 
     setattr(env, 'CVS', CVSFactory)
 
diff --git a/src/engine/SCons/Tool/Subversion.py b/src/engine/SCons/Tool/Subversion.py
index 56569f3..92839bc 100644
--- a/src/engine/SCons/Tool/Subversion.py
+++ b/src/engine/SCons/Tool/Subversion.py
@@ -48,8 +48,8 @@ def generate(env):
             module = os.path.join(module, '')
         return SCons.Builder.Builder(action = '$SVNCOM',
                                      env = env,
-                                     overrides = {'SVNREPOSITORY':repos,
-                                                  'SVNMODULE':module})
+                                     SVNREPOSITORY = repos,
+                                     SVNMODULE = module)
 
     setattr(env, 'Subversion', SubversionFactory)