Document function Actions and fix their return values (None == success, don't ignore failures).

8 files changed, 135 insertions, 13 deletions

diff --git a/doc/man/scons.1 b/doc/man/scons.1
index 6deab6e..28ad151 100644
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -723,6 +723,7 @@ to look-up a directory relative to the root of the source tree use #:
 env = Environment(CPPPATH='#/include')
 .EE
 
+.IP
 The directory look-up can also be forced using the 
 .BR Dir ()
 function:
@@ -1077,10 +1078,11 @@ method used to create an instance of the builder.
 The command line string used to build the target from the source. 
 .B action
 can also be a dictionary
-mapping source file name suffixes to command line string
+mapping source file name suffixes to
+any combination of command line strings
 (if the builder should accept multiple source file extensions),
-a Python function,
-or an Action object
+Python functions,
+or Action objects
 (see the next section).
 
 .IP prefix 
@@ -1110,7 +1112,7 @@ Builder objects,
 rather than let each separate Builder object
 create a separate Action.
 
-The Action function takes a single argument
+The Action method takes a single argument
 and returns an appropriate object for the action
 represented by the type of the argument:
 
@@ -1122,9 +1124,45 @@ the object is simply returned.
 If the argument is a string,
 a command-line Action is returned.
 
+.ES
+Action('$CC -c -o $TARGET $SOURCES')
+.EE
+
 .IP Function
 If the argument is a Python function,
 a function Action is returned.
+The Python function takes three keyword arguments,
+.B target
+(the name of the target file),
+.B source
+(the name of the source file)
+and
+.BR env
+(the construction environment
+used for building the target file).
+The
+.B target
+and
+.B source
+arguments may be lists of strings if there is
+more than one target file or source file.
+.IP
+The function should return
+.B 0
+or
+.B None
+to indicate a successful build of the target file(s).
+The function may raise an exception
+or return a non-zero exit status
+to indicate an unsuccessful build.
+
+.ES
+def build_it(target = None, source = None, env = None):
+    # build the target from the source
+    return 0
+ 
+a = Action(build_it)
+.EE
 
 .IP List
 If the argument is a list,
diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 2fa528e..70e30be 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -20,6 +20,9 @@ RELEASE 0.04 -
     variables and other functions, defined begin and end macros for
     the example sections, regularized white space separation.
 
+  - Function action fixes:  None is now a successful return value.
+    Exceptions are now reported.  Document function actions.
+
 
 
 RELEASE 0.03 - Fri, 11 Jan 2002 01:09:30 -0600
diff --git a/src/engine/SCons/Errors.py b/src/engine/SCons/Errors.py
index 2e8a455..ae2bbb9 100644
--- a/src/engine/SCons/Errors.py
+++ b/src/engine/SCons/Errors.py
@@ -33,9 +33,9 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 
 class BuildError(Exception):
-    def __init__(self, node=None, stat=0, *args):
+    def __init__(self, node=None, errstr="Unknown error", *args):
         self.node = node
-        self.stat = stat
+        self.errstr = errstr
         self.args = args
 
 class InternalError(Exception):
diff --git a/src/engine/SCons/ErrorsTests.py b/src/engine/SCons/ErrorsTests.py
index 2612c95..31ad119 100644
--- a/src/engine/SCons/ErrorsTests.py
+++ b/src/engine/SCons/ErrorsTests.py
@@ -32,10 +32,10 @@ class ErrorsTestCase(unittest.TestCase):
     def test_BuildError(self):
         """Test the BuildError exception."""
         try:
-            raise SCons.Errors.BuildError(node = "n", stat = 7)
+            raise SCons.Errors.BuildError(node = "n", errstr = "foo")
         except SCons.Errors.BuildError, e:
             assert e.node == "n"
-            assert e.stat == 7
+            assert e.errstr == "foo"
 
     def test_InternalError(self):
 	"""Test the InternalError exception."""
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index f55659e..37b9f37 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -47,10 +47,19 @@ class Builder:
     def get_contents(self, env, dir):
         return 7
 
+class NoneBuilder(Builder):
+    def execute(self, **kw):
+        apply(Builder.execute, (self,), kw)
+        return None
+
 class FailBuilder:
     def execute(self, **kw):
         return 1
 
+class ExceptBuilder:
+    def execute(self, **kw):
+        raise SCons.Errors.BuildError
+
 class Environment:
     def Dictionary(self, *args):
 	pass
@@ -72,9 +81,21 @@ class NodeTestCase(unittest.TestCase):
 	else:
 	    raise TestFailed, "did not catch expected BuildError"
 
+        node = SCons.Node.Node()
+        node.builder_set(ExceptBuilder())
+        node.env_set(Environment())
+        try:
+            node.build()
+        except SCons.Errors.BuildError:
+            pass
+        else:
+            raise TestFailed, "did not catch expected BuildError"
+
     def test_build(self):
 	"""Test building a node
 	"""
+        global built_it
+
         class MyNode(SCons.Node.Node):
             def __str__(self):
                 return self.path
@@ -94,6 +115,18 @@ class NodeTestCase(unittest.TestCase):
         assert str(built_target) == "xxx", str(built_target)
         assert built_source == ["yyy", "zzz"], built_source
 
+        built_it = None
+        node = MyNode()
+        node.builder_set(NoneBuilder())
+        node.env_set(Environment())
+        node.path = "qqq"
+        node.sources = ["rrr", "sss"]
+        node.build()
+        assert built_it
+        assert type(built_target) == type(MyNode()), type(built_target)
+        assert str(built_target) == "qqq", str(built_target)
+        assert built_source == ["rrr", "sss"], built_source
+
     def test_builder_set(self):
 	"""Test setting a Node's Builder
 	"""
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index b81f911..102b532 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -72,10 +72,13 @@ class Node:
         """Actually build the node.   Return the status from the build."""
 	if not self.builder:
 	    return None
-        stat = self.builder.execute(env = self.env.Dictionary(),
-                                    target = self, source = self.sources)
-	if stat != 0:
-	    raise BuildError(node = self, stat = stat)
+        try:
+            stat = self.builder.execute(env = self.env.Dictionary(),
+                                        target = self, source = self.sources)
+        except:
+            raise BuildError(node = self, errstr = "Exception")
+        if stat:
+            raise BuildError(node = self, errstr = "Error %d" % stat)
 
         # If we succesfully build a node, then we need to rescan for
         # implicit dependencies, since it might have changed on us.
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index 101e5d6..1711f7a 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -75,7 +75,7 @@ class BuildTask(SCons.Taskmaster.Task):
             try:
                 self.target.build()
             except BuildError, e:
-                sys.stderr.write("scons: *** [%s] Error %s\n" % (e.node, str(e.stat)))
+                sys.stderr.write("scons: *** [%s] %s\n" % (e.node, e.errstr))
                 raise
 
     def executed(self):
diff --git a/test/exceptions.py b/test/exceptions.py
new file mode 100644
index 0000000..e67cec6
--- /dev/null
+++ b/test/exceptions.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+#
+# Copyright (c) 2001 Steven Knight
+#
+# 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__"
+
+import os
+import sys
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+test.write('SConstruct', """
+def func(source = None, target = None, env = None):
+    raise "func exception"
+B = Builder(name = 'B', action = func)
+env = Environment(BUILDERS = [B])
+env.B(target = 'foo.out', source = 'foo.in')
+""")
+
+test.write('foo.in', "foo.in\n")
+
+test.run(arguments = "foo.out", stderr = "scons: *** [foo.out] Exception\n")
+
+test.pass_test()