Apply build overrides once up front. (Kevin Quick)

5 files changed, 64 insertions, 21 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 41a96ce..9f6328f 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -331,7 +331,8 @@ RELEASE 0.97 - XXX
   - Fix command-line expansion of Python Value Nodes.
 
   - Internal cleanups:  Remove an unnecessary scan argument.  Associate
-    Scanners only with Builders, not nodes.
+    Scanners only with Builders, not nodes.  Apply overrides once when a
+    Builder is called, not in multiple places.
 
   - Use the correct scanner if the same source file is used for targets in
     two different environments with the same path but different scanners.
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 04dd1e5..ed070c4 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -298,9 +298,6 @@ def _init_nodes(builder, env, overrides, executor_kw, tlist, slist):
                 else:
                     raise UserError, "Two environments with different actions were specified for the same target: %s"%str(t)
 
-            elif t.overrides != overrides:
-                raise UserError, "Two different sets of overrides were specified for the same target: %s"%str(t)
-
             if builder.multi:
                 if t.builder != builder:
                     if isinstance(t.builder, ListBuilder) and isinstance(builder, ListBuilder) and t.builder.builder == builder.builder:
@@ -332,14 +329,13 @@ def _init_nodes(builder, env, overrides, executor_kw, tlist, slist):
             raise UserError, "Builder %s must have an action to build %s."%(builder.get_name(env or builder.env), map(str,tlist))
         executor = SCons.Executor.Executor(builder.action,
                                            env or builder.env,
-                                           [builder.overrides, overrides],
+                                           [],  # env already has overrides
                                            tlist,
                                            slist,
                                            executor_kw)
 
     # Now set up the relevant information in the target Nodes themselves.
     for t in tlist:
-        t.overrides = overrides
         t.cwd = SCons.Node.FS.default_fs.getcwd()
         t.builder_set(builder)
         t.env_set(env)
@@ -491,8 +487,6 @@ class BuilderBase:
 
         overwarn.warn()
 
-        env = env.Override(overwarn.data)
-
         src_suf = self.get_src_suffix(env)
 
         source = _adjustixes(source, None, src_suf)
@@ -572,6 +566,15 @@ class BuilderBase:
         else:
             ekw = self.executor_kw.copy()
             ekw['chdir'] = chdir
+        if kw:
+            if self.overrides:
+                env_kw = self.overrides.copy()
+                env_kw.update(kw)
+            else:
+                env_kw = kw
+        else:
+            env_kw = self.overrides
+        env = env.Override(env_kw)
         return self._execute(env, target, source, OverrideWarner(kw), ekw)
 
     def adjust_suffix(self, suff):
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index 5ed377c..c829f22 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -137,7 +137,6 @@ class BuildDirTestCase(unittest.TestCase):
         fs.BuildDir('../var2', 'src')
         f1 = fs.File('../var1/test1')
         f2 = fs.File('../var2/test1')
-        assert hasattr(f1, 'overrides')
         assert f1.srcnode().path == os.path.normpath('src/test1'), f1.srcnode().path
         assert f2.srcnode().path == os.path.normpath('src/test1'), f2.srcnode().path
 
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 24ea2a9..767b9e3 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -126,7 +126,6 @@ class Node:
         self.always_build = None
         self.found_includes = {}
         self.includes = None
-        self.overrides = {}     # construction variable overrides for building this node
         self.attributes = self.Attrs() # Generic place to stick information about the Node.
         self.side_effect = 0 # true iff this node is a side effect
         self.side_effects = [] # the side effects of building this target
diff --git a/test/multi.py b/test/multi.py
index 8e590d9..7ae1736 100644
--- a/test/multi.py
+++ b/test/multi.py
@@ -87,8 +87,8 @@ scons: *** Multiple ways to build the same target were specified for: file2.out
 
 
 #
-# The second call generates an error if the two calls have different
-# overrides.
+# A warning is generated if the calls have different overrides but the
+# overrides don't appear to affect the build operation.
 #
 
 test.write('SConstruct', """
@@ -107,34 +107,75 @@ test.write('file3a.in', 'file3a.in\n')
 test.write('file3b.in', 'file3b.in\n')
 
 test.run(arguments='file3.out', 
-         status=2, 
          stderr=TestSCons.re_escape("""
-scons: *** Two different sets of overrides were specified for the same target: file3.out
+scons: warning: Two different environments were specified for target file3.out,
+\tbut they appear to have the same action: build(target, source, env)
 """) + TestSCons.file_expr)
 
+#
+# A warning is generated if the calls have different overrides but the
+# overrides don't appear to affect the build operation.
+#
+
+test.write('build.py',r"""#!/usr/bin/env python
+import sys
+def build(num, target, source):
+    file = open(str(target), 'wb')
+    file.write('%s\n'%num)
+    for s in source:
+        file.write(open(str(s), 'rb').read())
+build(sys.argv[1],sys.argv[2],sys.argv[3:])
+""")
+
+test.write('SConstruct', """
+
+B = Builder(action='%(python)s build.py $foo $TARGET $SOURCES', multi=1)
+env = Environment(BUILDERS = { 'B' : B })
+env.B(target = 'file03.out', source = 'file03a.in', foo=1)
+env.B(target = 'file03.out', source = 'file03b.in', foo=2)
+""" % {'python':TestSCons.python})
+
+test.write('file03a.in', 'file03a.in\n')
+test.write('file03b.in', 'file03b.in\n')
+
+test.run(arguments='file03.out', 
+         status=2, 
+         stderr=TestSCons.re_escape("""
+scons: *** Two environments with different actions were specified for the same target: file03.out
+""") + TestSCons.file_expr)
 
 #
 # Everything works if the two calls have the same overrides.
 #
 
-test.write('SConstruct', """
-def build(env, target, source):
-    file = open(str(target[0]), 'wb')
+test.write('build.py',r"""#!/usr/bin/env python
+import sys
+def build(num, target, source):
+    file = open(str(target), 'wb')
+    file.write('%s\n'%num)
     for s in source:
         file.write(open(str(s), 'rb').read())
+build(sys.argv[1],sys.argv[2],sys.argv[3:])
+""")
 
-B = Builder(action=build, multi=1)
+test.write('SConstruct', """
+
+B = Builder(action='%(python)s build.py $foo $TARGET $SOURCES', multi=1)
 env = Environment(BUILDERS = { 'B' : B })
 env.B(target = 'file4.out', source = 'file4a.in', foo=3)
 env.B(target = 'file4.out', source = 'file4b.in', foo=3)
-""")
+""" % {'python':TestSCons.python})
 
 test.write('file4a.in', 'file4a.in\n')
 test.write('file4b.in', 'file4b.in\n')
 
-test.run(arguments='file4.out')
+test.run(arguments='file4.out', 
+         stderr=TestSCons.re_escape("""
+scons: warning: Two different environments were specified for target file4.out,
+	but they appear to have the same action: %(python)s build.py \$foo \$TARGET \$SOURCES
+""" % {'python':TestSCons.python}) + TestSCons.file_expr)
 
-test.must_match('file4.out', "file4a.in\nfile4b.in\n")
+test.must_match('file4.out', "3\nfile4a.in\nfile4b.in\n")
 
 
 #