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

5 files changed, 31 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"""