Better error messages for bad builder creation. (Kevin Quick)

6 files changed, 130 insertions, 6 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 9ba813b..502ef2a 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -132,6 +132,9 @@ RELEASE 0.97 - XXX
   - Fix handling when BuildDir() exists but is unwriteable.  Add
     "Stop." to those error messages for consistency.
 
+  - Catch incidents of bad builder creation (without an action) and
+    supply meaningful error messages.
+
   From Christoph Wiedemann:
 
   - Add an Environment.SetDefault() method that only sets values if
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 543ca8e..c1c7b49 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -329,6 +329,8 @@ def _init_nodes(builder, env, overrides, tlist, slist):
         else:
             executor.add_sources(slist)
     if executor is None:
+        if not builder.action:
+            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],
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index e4647f5..9c5b3e5 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -265,6 +265,15 @@ class BuilderTestCase(unittest.TestCase):
         target = builder(env, source='n21')[0]
         assert target.name == 'p-n21.s', target
 
+        builder = SCons.Builder.Builder(misspelled_action="foo",
+                                        suffix = '.s')
+        try:
+            builder(env, target = 'n22', source = 'n22')
+        except SCons.Errors.UserError, e:
+            pass
+        else:
+            raise "Did not catch expected UserError."
+
     def test_mistaken_variables(self):
         """Test keyword arguments that are often mistakes
         """
@@ -393,7 +402,7 @@ class BuilderTestCase(unittest.TestCase):
         env = Environment()
         builder = SCons.Builder.Builder(prefix = 'lib.')
         assert builder.get_prefix(env) == 'lib.'
-        builder = SCons.Builder.Builder(prefix = 'lib')
+        builder = SCons.Builder.Builder(prefix = 'lib', action='')
         assert builder.get_prefix(env) == 'lib'
         tgt = builder(env, target = 'tgt1', source = 'src1')[0]
         assert tgt.path == 'libtgt1', \
@@ -426,7 +435,8 @@ class BuilderTestCase(unittest.TestCase):
                                                   '.in'  : 'out-',
                                                   '.x'   : 'y-',
                                                   '$FOO' : 'foo-',
-                                                  '.zzz' : my_emit})
+                                                  '.zzz' : my_emit},
+                                        action = '')
         tgt = builder(my_env, source = 'f1')[0]
         assert tgt.path == 'default-f1', tgt.path
         tgt = builder(my_env, source = 'f2.c')[0]
@@ -448,7 +458,7 @@ class BuilderTestCase(unittest.TestCase):
         """
         env = Environment(XSUFFIX = '.x', YSUFFIX = '.y')
 
-        b1 = SCons.Builder.Builder(src_suffix = '.c')
+        b1 = SCons.Builder.Builder(src_suffix = '.c', action='')
         assert b1.src_suffixes(env) == ['.c'], b1.src_suffixes(env)
 
         tgt = b1(env, target = 'tgt2', source = 'src2')[0]
@@ -483,7 +493,7 @@ class BuilderTestCase(unittest.TestCase):
         env = Environment()
         builder = SCons.Builder.Builder(suffix = '.o')
         assert builder.get_suffix(env) == '.o', builder.get_suffix(env)
-        builder = SCons.Builder.Builder(suffix = 'o')
+        builder = SCons.Builder.Builder(suffix = 'o', action='')
         assert builder.get_suffix(env) == '.o', builder.get_suffix(env)
         tgt = builder(env, target = 'tgt3', source = 'src3')[0]
         assert tgt.path == 'tgt3.o', \
@@ -510,7 +520,8 @@ class BuilderTestCase(unittest.TestCase):
                                                   '.in'  : '.out',
                                                   '.x'   : '.y',
                                                   '$BAR' : '.new',
-                                                  '.zzz' : my_emit})
+                                                  '.zzz' : my_emit},
+                                        action='')
         tgt = builder(my_env, source = 'f1')[0]
         assert tgt.path == 'f1.default', tgt.path
         tgt = builder(my_env, source = 'f2.c')[0]
@@ -809,7 +820,8 @@ class BuilderTestCase(unittest.TestCase):
         sscan = TestScanner()
         env = Environment()
         builder = SCons.Builder.Builder(target_scanner=tscan,
-                                        source_scanner=sscan)
+                                        source_scanner=sscan,
+                                        action='')
         tgt = builder(env, target='foo2', source='bar')[0]
         assert tgt.target_scanner == tscan, tgt.target_scanner
         assert tgt.source_scanner == sscan, tgt.source_scanner
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index bb10ea0..b9d9897 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -50,6 +50,8 @@ class Executor:
         self.overridelist = overridelist
         self.targets = targets
         self.sources = sources[:]
+        if not action:
+            raise SCons.Errors.UserError, "Executor must have an action."
 
     def get_build_env(self):
         """Fetch or create the appropriate build Environment
diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py
index c391543..cfa2dcd 100644
--- a/src/engine/SCons/ExecutorTests.py
+++ b/src/engine/SCons/ExecutorTests.py
@@ -81,6 +81,12 @@ class ExecutorTestCase(unittest.TestCase):
         assert x.targets == 't', x.targets
         source_list.append('s3')
         assert x.sources == ['s1', 's2'], x.sources
+        try:
+            x = SCons.Executor.Executor(None, 'e', ['o'], 't', source_list)
+        except SCons.Errors.UserError:
+            pass
+        else:
+            raise "Did not catch expected UserError"
 
     def test_get_build_env(self):
         """Test fetching and generating a build environment"""
diff --git a/test/BadBuilder.py b/test/BadBuilder.py
new file mode 100644
index 0000000..4667a21
--- /dev/null
+++ b/test/BadBuilder.py
@@ -0,0 +1,99 @@
+#!/usr/bin/env python
+#
+# __COPYRIGHT__
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+#
+
+__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
+
+"""
+Test the ability to catch Builder creation with poorly specified Actions.
+"""
+
+import os.path
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+sconstruct = """
+def buildop(env, source, target):
+    outf = open(str(target[0]), 'wb')
+    inpf = open(str(source[0]), 'r')
+    for line in inpf.readlines():
+        if line.find(str(target[0])) == -1:
+            outf.write(line)
+    inpf.close()
+    outf.close()
+b1 = Builder(action=buildop, src_suffix='.a', suffix='.b')
+%s
+env=Environment(tools=[], BUILDERS={'b1':b1, 'b2':b2})
+foo_b = env.b1(source='foo.a')
+env.b2(source=foo_b)
+"""
+
+test.write('foo.a', """\
+foo.c
+foo.b
+built
+""")
+
+### Gross mistake in Builder spec
+
+test.write('SConstruct', sconstruct % '\
+b2 = Builder(act__ion=buildop, src_suffix=".b", suffix=".c")')
+
+test.run(arguments='.',
+         stderr="""\
+
+scons: *** Builder b2 must have an action to build ['foo.c'].
+File "SConstruct", line 14, in ?
+""",
+status = 2)
+
+### Subtle mistake in Builder spec
+
+test.write('SConstruct', sconstruct % '\
+b2 = Builder(actoin=buildop, src_suffix=".b", suffix=".c")')
+
+test.run(arguments='test2',
+         stderr="""\
+
+scons: *** Builder b2 must have an action to build ['foo.c'].
+File "SConstruct", line 14, in ?
+""",
+status = 2)
+
+### Missing action in Builder spec
+
+test.write('SConstruct', sconstruct % '\
+b2 = Builder(src_suffix=".b", suffix=".c")')
+
+test.run(arguments='test2',
+         stderr="""\
+
+scons: *** Builder b2 must have an action to build ['foo.c'].
+File "SConstruct", line 14, in ?
+""",
+status = 2)
+
+
+test.pass_test()