Refactor Action/Executor interaction. (Kevin Quick)

8 files changed, 290 insertions, 298 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 1995a0b..579811e 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -238,6 +238,23 @@ RELEASE 0.97 - XXX
     Command() and Scanner test coverage.  Improved test infrastructure
     for -c output.
 
+  - Refactor the interface between Action and Executor objects to treat
+    Actions atomically.
+
+  - The --debug=presub option will now report the pre-substitution
+    each action seprately, instead of reporting the entire list before
+    executing the actions one by one.
+
+  - The --debug=explain option explaining a changed action will now
+    (more correctly) show pre-substitution action strings, instead of
+    the commands with substituted file names.
+
+  - A Node (file) will now be rebuilt if its PreAction or PostAction
+    actions change.
+
+  - Python Function actions now have their calling signature (target,
+    source, env) reported correctly when displayed.
+
   From Levi Stephen:
 
   - Allow $JARCHDIR to be expanded to other construction variables.
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index ba1c240..5f89cc2 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -8,6 +8,11 @@ The base class here is ActionBase.  The base class supplies just a few
 OO utility methods and some generic methods for displaying information
 about an Action in response to the various commands that control printing.
 
+A second-level base class is _ActionAction.  This extends ActionBase
+by providing the methods that can be used to show and perform an
+action.  True Action objects will subclass _ActionAction; Action
+factory class objects will subclass ActionBase.
+
 The heavy lifting is handled by subclasses for the different types of
 actions we might execute:
 
@@ -30,27 +35,21 @@ other modules:
         needs to be rebuilt because its action changed.
 
     genstring()
-        Returns a string representation of the Action *without* command
-        substitution, but allows a CommandGeneratorAction to generate
-        the right action based on the specified target, source and env.
-        This is used by the Signature subsystem (through the Executor)
-        to compare the actions used to build a target last time and
-        this time.
+        Returns a string representation of the Action *without*
+        command substitution, but allows a CommandGeneratorAction to
+        generate the right action based on the specified target,
+        source and env.  This is used by the Signature subsystem
+        (through the Executor) to obtain an (imprecise) representation
+        of the Action operation for informative purposes.
 
-    strfunction()
-        Returns a substituted string representation of the Action.
-        This is used by the ActionBase.show() command to display the
-        command/function that will be executed to generate the target(s).
 
 Subclasses also supply the following methods for internal use within
 this module:
 
     __str__()
-        Returns a string representation of the Action *without* command
-        substitution.  This is used by the __call__() methods to display
-        the pre-substitution command whenever the --debug=presub option
-        is used.
-
+        Returns a string approximation of the Action; no variable
+        substitution is performed.
+        
     execute()
         The internal method that really, truly, actually handles the
         execution of a command or Python function.  This is used so
@@ -58,6 +57,11 @@ this module:
         pre-substitution representations, and *then* execute an action
         without worrying about the specific Actions involved.
 
+    strfunction()
+        Returns a substituted string representation of the Action.
+        This is used by the _ActionAction.show() command to display the
+        command/function that will be executed to generate the target(s).
+
 There is a related independent ActionCaller class that looks like a
 regular Action, and which serves as a wrapper for arbitrary functions
 that we want to let the user specify the arguments to now, but actually
@@ -187,8 +191,10 @@ def _do_create_action(act, *args, **kw):
         if len(commands) == 1:
             return apply(CommandAction, (commands[0],)+args, kw)
         else:
-            listCmdActions = map(lambda x: CommandAction(x), commands)
-            return apply(ListAction, (listCmdActions,)+args, kw)
+            listCmdActions = map(lambda x, args=args, kw=kw:
+                                 apply(CommandAction, (x,)+args, kw),
+                                 commands)
+            return ListAction(listCmdActions)
     return None
 
 def Action(act, *args, **kw):
@@ -201,11 +207,41 @@ def Action(act, *args, **kw):
         if len(acts) == 1:
             return acts[0]
         else:
-            return apply(ListAction, (acts,)+args, kw)
+            return ListAction(acts)
     else:
         return apply(_do_create_action, (act,)+args, kw)
 
 class ActionBase:
+    """Base class for all types of action objects that can be held by
+    other objects (Builders, Executors, etc.)  This provides the
+    common methods for manipulating and combining those actions."""
+    
+    def __cmp__(self, other):
+        return cmp(self.__dict__, other)
+
+    def genstring(self, target, source, env):
+        return str(self)
+
+    def __add__(self, other):
+        return _actionAppend(self, other)
+
+    def __radd__(self, other):
+        return _actionAppend(other, self)
+
+    def presub_lines(self, env):
+        # CommandGeneratorAction needs a real environment
+        # in order to return the proper string here, since
+        # it may call LazyCmdGenerator, which looks up a key
+        # in that env.  So we temporarily remember the env here,
+        # and CommandGeneratorAction will use this env
+        # when it calls its __generate method.
+        self.presub_env = env
+        lines = string.split(str(self), '\n')
+        self.presub_env = None      # don't need this any more
+        return lines
+
+
+class _ActionAction(ActionBase):
     """Base class for actions that create output objects."""
     def __init__(self, strfunction=_null, presub=_null, chdir=None, **kw):
         if not strfunction is _null:
@@ -215,9 +251,6 @@ class ActionBase:
         self.presub = presub
         self.chdir = chdir
 
-    def __cmp__(self, other):
-        return cmp(self.__dict__, other)
-
     def print_cmd_line(self, s, target, source, env):
         sys.stdout.write(s + "\n")
 
@@ -244,9 +277,9 @@ class ActionBase:
                 if not SCons.Util.is_String(chdir):
                     chdir = str(target[0].dir)
         if presub:
-            t = string.join(map(str, target), 'and')
+            t = string.join(map(str, target), ' and ')
             l = string.join(self.presub_lines(env), '\n  ')
-            out = "Building %s with action(s):\n  %s\n" % (t, l)
+            out = "Building %s with action:\n  %s\n" % (t, l)
             sys.stdout.write(out)
         s = None
         if show and self.strfunction:
@@ -278,26 +311,6 @@ class ActionBase:
             print_func('os.chdir(%s)' % repr(save_cwd), target, source, env)
         return stat
 
-    def presub_lines(self, env):
-        # CommandGeneratorAction needs a real environment
-        # in order to return the proper string here, since
-        # it may call LazyCmdGenerator, which looks up a key
-        # in that env.  So we temporarily remember the env here,
-        # and CommandGeneratorAction will use this env
-        # when it calls its __generate method.
-        self.presub_env = env
-        lines = string.split(str(self), '\n')
-        self.presub_env = None      # don't need this any more
-        return lines
-
-    def genstring(self, target, source, env):
-        return str(self)
-
-    def __add__(self, other):
-        return _actionAppend(self, other)
-
-    def __radd__(self, other):
-        return _actionAppend(other, self)
 
 def _string_from_cmd_list(cmd_list):
     """Takes a list of command line arguments and returns a pretty
@@ -309,22 +322,34 @@ def _string_from_cmd_list(cmd_list):
         cl.append(arg)
     return string.join(cl)
 
-class CommandAction(ActionBase):
+class CommandAction(_ActionAction):
     """Class for command-execution actions."""
     def __init__(self, cmd, *args, **kw):
-        # Cmd list can actually be a list or a single item...basically
-        # anything that we could pass in as the first arg to
-        # Environment.subst_list().
+        # Cmd can actually be a list or a single item; if it's a
+        # single item it should be the command string to execute; if a
+        # list then it should be the words of the command string to
+        # execute.  Only a single command should be executed by this
+        # object; lists of commands should be handled by embedding
+        # these objects in a ListAction object (which the Action()
+        # factory above does).  cmd will be passed to
+        # Environment.subst_list() for substituting environment
+        # variables.
         if __debug__: logInstanceCreation(self)
-        apply(ActionBase.__init__, (self,)+args, kw)
+        apply(_ActionAction.__init__, (self,)+args, kw)
+        if SCons.Util.is_List(cmd):
+            if filter(SCons.Util.is_List, cmd):
+                raise TypeError, "CommandAction should be given only " \
+                      "a single command"
         self.cmd_list = cmd
 
     def __str__(self):
+        if SCons.Util.is_List(self.cmd_list):
+            return string.join(map(str, self.cmd_list), ' ')
         return str(self.cmd_list)
 
     def strfunction(self, target, source, env):
         cmd_list = env.subst_list(self.cmd_list, 0, target, source)
-        return string.join(map(_string_from_cmd_list, cmd_list), "\n")
+        return _string_from_cmd_list(cmd_list[0])
 
     def execute(self, target, source, env):
         """Execute a command action.
@@ -374,7 +399,8 @@ class CommandAction(ActionBase):
                     # reasonable for just about everything else:
                     ENV[key] = str(value)
 
-        cmd_list = env.subst_list(self.cmd_list, 0, target, source)
+        cmd_list = env.subst_list(self.cmd_list, 0, target,
+                                  map(rfile, source))
 
         # Use len() to filter out any "command" that's zero-length.
         for cmd_line in filter(len, cmd_list):
@@ -402,8 +428,8 @@ class CommandGeneratorAction(ActionBase):
     """Class for command-generator actions."""
     def __init__(self, generator, *args, **kw):
         if __debug__: logInstanceCreation(self)
-        apply(ActionBase.__init__, (self,)+args, kw)
         self.generator = generator
+        self.gen_kw = kw
 
     def __generate(self, target, source, env, for_signature):
         # ensure that target is a list, to make it easier to write
@@ -412,36 +438,27 @@ class CommandGeneratorAction(ActionBase):
             target = [target]
 
         ret = self.generator(target=target, source=source, env=env, for_signature=for_signature)
-        gen_cmd = Action(ret)
+        gen_cmd = apply(Action, (ret,), self.gen_kw)
         if not gen_cmd:
             raise SCons.Errors.UserError("Object returned from command generator: %s cannot be used to create an Action." % repr(ret))
         return gen_cmd
 
-    def strfunction(self, target, source, env):
-        if not SCons.Util.is_List(source):
-            source = [source]
-        rsources = map(rfile, source)
-        act = self.__generate(target, source, env, 0)
-        if act.strfunction:
-            return act.strfunction(target, rsources, env)
-        else:
-            return None
-
     def __str__(self):
         try:
             env = self.presub_env or {}
         except AttributeError:
             env = {}
-        act = self.__generate([], [], env, 0)
+        act = self.__generate([], [], env, 1)
         return str(act)
 
     def genstring(self, target, source, env):
-        return str(self.__generate(target, source, env, 0))
+        return self.__generate(target, source, env, 1).genstring(target, source, env)
 
-    def execute(self, target, source, env):
-        rsources = map(rfile, source)
+    def __call__(self, target, source, env, errfunc=None, presub=_null,
+                 show=_null, execute=_null, chdir=_null):
         act = self.__generate(target, source, env, 0)
-        return act.execute(target, source, env)
+        return act(target, source, env, errfunc, presub,
+                   show, execute, chdir)
 
     def get_contents(self, target, source, env, dict=None):
         """Return the signature contents of this action's command line.
@@ -461,13 +478,6 @@ class LazyCmdGenerator:
         if __debug__: logInstanceCreation(self)
         self.var = SCons.Util.to_String(var)
 
-    def strfunction(self, target, source, env):
-        try:
-            return env[self.var]
-        except KeyError:
-            # The variable reference substitutes to nothing.
-            return ''
-
     def __str__(self):
         return 'LazyCmdGenerator: %s'%str(self.var)
 
@@ -481,13 +491,13 @@ class LazyCmdGenerator:
     def __cmp__(self, other):
         return cmp(self.__dict__, other)
 
-class FunctionAction(ActionBase):
+class FunctionAction(_ActionAction):
     """Class for Python function actions."""
 
     def __init__(self, execfunction, *args, **kw):
         if __debug__: logInstanceCreation(self)
         self.execfunction = execfunction
-        apply(ActionBase.__init__, (self,)+args, kw)
+        apply(_ActionAction.__init__, (self,)+args, kw)
         self.varlist = kw.get('varlist', [])
 
     def function_name(self):
@@ -519,7 +529,7 @@ class FunctionAction(ActionBase):
         return "%s(%s, %s)" % (name, tstr, sstr)
 
     def __str__(self):
-        return "%s(env, target, source)" % self.function_name()
+        return "%s(target, source, env)" % self.function_name()
 
     def execute(self, target, source, env):
         rsources = map(rfile, source)
@@ -557,33 +567,21 @@ class FunctionAction(ActionBase):
 
 class ListAction(ActionBase):
     """Class for lists of other actions."""
-    def __init__(self, list, *args, **kw):
+    def __init__(self, list):
         if __debug__: logInstanceCreation(self)
-        apply(ActionBase.__init__, (self,)+args, kw)
-        self.list = map(lambda x: Action(x), list)
+        def list_of_actions(x):
+            if isinstance(x, ActionBase):
+                return x
+            return Action(x)
+        self.list = map(list_of_actions, list)
 
     def __str__(self):
-        s = []
-        for l in self.list:
-            s.append(str(l))
-        return string.join(s, "\n")
-
-    def strfunction(self, target, source, env):
-        s = []
-        for l in self.list:
-            if l.strfunction:
-                x = l.strfunction(target, source, env)
-                if not SCons.Util.is_List(x):
-                    x = [x]
-                s.extend(x)
-        return string.join(s, "\n")
-
-    def execute(self, target, source, env):
-        for l in self.list:
-            r = l.execute(target, source, env)
-            if r:
-                return r
-        return 0
+        return string.join(map(str, self.list), '\n')
+    
+    def presub_lines(self, env):
+        return SCons.Util.flatten(map(lambda a, env=env:
+                                      a.presub_lines(env),
+                                      self.list))
 
     def get_contents(self, target, source, env, dict=None):
         """Return the signature contents of this action list.
@@ -596,6 +594,15 @@ class ListAction(ActionBase):
                                self.list),
                            "")
 
+    def __call__(self, target, source, env, errfunc=None, presub=_null,
+                 show=_null, execute=_null, chdir=_null):
+        for act in self.list:
+            stat = act(target, source, env, errfunc, presub,
+                       show, execute, chdir)
+            if stat:
+                return stat
+        return 0
+
 class ActionCaller:
     """A class for delaying calling an Action function with specific
     (positional and keyword) arguments until the Action is actually
@@ -655,4 +662,12 @@ class ActionFactory:
         self.strfunc = strfunc
     def __call__(self, *args, **kw):
         ac = ActionCaller(self, args, kw)
-        return Action(ac, strfunction=ac.strfunction)
+        action = Action(ac, strfunction=ac.strfunction)
+        # action will be a FunctionAction; if left to its own devices,
+        # a genstr or str of this action will just show
+        # "ActionCaller(target, source, env)".  Override that with the
+        # description from strfunc.  Note that the apply is evaluated
+        # right now; __str__ is set to a (lambda) function that just
+        # returns the stored result of the evaluation whenever called.
+        action.__str__ = lambda name=apply(self.strfunc, args, kw): name
+        return action
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 6bff0e3..e996f61 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -260,17 +260,19 @@ class ActionTestCase(unittest.TestCase):
         assert isinstance(a4, SCons.Action.ListAction), a4
         assert isinstance(a4.list[0], SCons.Action.CommandAction), a4.list[0]
         assert a4.list[0].cmd_list == "x", a4.list[0].cmd_list
+        assert a4.list[0].strfunction == foo, a4.list[0].strfunction
         assert isinstance(a4.list[1], SCons.Action.CommandAction), a4.list[1]
         assert a4.list[1].cmd_list == "y", a4.list[1].cmd_list
-        assert a4.strfunction == foo, a4.strfunction
+        assert a4.list[1].strfunction == foo, a4.list[1].strfunction
 
         a5 = SCons.Action.Action("x\ny", strfunction=foo)
         assert isinstance(a5, SCons.Action.ListAction), a5
         assert isinstance(a5.list[0], SCons.Action.CommandAction), a5.list[0]
         assert a5.list[0].cmd_list == "x", a5.list[0].cmd_list
+        assert a5.list[0].strfunction == foo, a5.list[0].strfunction
         assert isinstance(a5.list[1], SCons.Action.CommandAction), a5.list[1]
         assert a5.list[1].cmd_list == "y", a5.list[1].cmd_list
-        assert a5.strfunction == foo, a5.strfunction
+        assert a5.list[1].strfunction == foo, a5.list[1].strfunction
 
     def test_CommandGeneratorAction(self):
         """Test the Action() factory's creation of CommandGeneratorAction objects
@@ -288,7 +290,6 @@ class ActionTestCase(unittest.TestCase):
         a2 = SCons.Action.Action(cg, strfunction=bar)
         assert isinstance(a2, SCons.Action.CommandGeneratorAction), a2
         assert a2.generator is foo, a2.generator
-        assert a2.strfunction is bar, a2.strfunction
 
     def test_LazyCmdGeneratorAction(self):
         """Test the Action() factory's creation of lazy CommandGeneratorAction objects
@@ -303,7 +304,6 @@ class ActionTestCase(unittest.TestCase):
         a2 = SCons.Action.Action("$FOO", strfunction=foo)
         assert isinstance(a2, SCons.Action.CommandGeneratorAction), a2
         assert isinstance(a2.generator, SCons.Action.LazyCmdGenerator), a2.generator
-        assert a2.strfunction is foo, a2.strfunction
 
     def test_no_action(self):
         """Test when the Action() factory can't create an action object
@@ -319,9 +319,14 @@ class ActionTestCase(unittest.TestCase):
         assert a2 is a1, a2
 
 class ActionBaseTestCase(unittest.TestCase):
+    # Maybe write this in the future...
+    pass
+ 
+class _ActionActionTestCase(unittest.TestCase):
+    
 
     def test__init__(self):
-        """Test creation of ActionBase objects
+        """Test creation of _ActionAction objects
         """
 
         def func1():
@@ -330,23 +335,23 @@ class ActionBaseTestCase(unittest.TestCase):
         def func2():
             pass
 
-        a = SCons.Action.ActionBase()
+        a = SCons.Action._ActionAction()
         assert not hasattr(a, 'strfunction')
 
-        assert SCons.Action.ActionBase(kwarg = 1)
+        assert SCons.Action._ActionAction(kwarg = 1)
         assert not hasattr(a, 'strfunction')
         assert not hasattr(a, 'kwarg')
 
-        a = SCons.Action.ActionBase(strfunction=func1)
+        a = SCons.Action._ActionAction(strfunction=func1)
         assert a.strfunction is func1, a.strfunction
 
-        a = SCons.Action.ActionBase(presub=func1)
+        a = SCons.Action._ActionAction(presub=func1)
         assert a.presub is func1, a.presub
 
-        a = SCons.Action.ActionBase(chdir=1)
+        a = SCons.Action._ActionAction(chdir=1)
         assert a.chdir is 1, a.chdir
 
-        a = SCons.Action.ActionBase(func1, func2, 'x')
+        a = SCons.Action._ActionAction(func1, func2, 'x')
         assert a.strfunction is func1, a.strfunction
         assert a.presub is func2, a.presub
         assert a.chdir is 'x', a.chdir
@@ -403,6 +408,16 @@ class ActionBaseTestCase(unittest.TestCase):
                 return 7
             a = SCons.Action.Action(execfunc)
 
+            def firstfunc(target, source, env):
+                assert type(target) is type([]), type(target)
+                assert type(source) is type([]), type(source)
+                return 0
+            def lastfunc(target, source, env):
+                assert type(target) is type([]), type(target)
+                assert type(source) is type([]), type(source)
+                return 9
+            b = SCons.Action.Action([firstfunc, execfunc, lastfunc])
+            
             sio = StringIO.StringIO()
             sys.stdout = sio
             result = a("out", "in", env)
@@ -429,6 +444,13 @@ class ActionBaseTestCase(unittest.TestCase):
 
             a.chdir = None
 
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = b("out", "in", env)
+            assert result == 7, result
+            s = sio.getvalue()
+            assert s == 'firstfunc(["out"], ["in"])\nexecfunc(["out"], ["in"])\n', s
+
             SCons.Action.execute_actions = 0
 
             sio = StringIO.StringIO()
@@ -438,30 +460,61 @@ class ActionBaseTestCase(unittest.TestCase):
             s = sio.getvalue()
             assert s == 'execfunc(["out"], ["in"])\n', s
 
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = b("out", "in", env)
+            assert result == 0, result
+            s = sio.getvalue()
+            assert s == 'firstfunc(["out"], ["in"])\nexecfunc(["out"], ["in"])\nlastfunc(["out"], ["in"])\n', s
+
             SCons.Action.print_actions_presub = 1
+            SCons.Action.execute_actions = 1
 
             sio = StringIO.StringIO()
             sys.stdout = sio
             result = a("out", "in", env)
-            assert result == 0, result
+            assert result == 7, result
             s = sio.getvalue()
             assert s == 'execfunc(["out"], ["in"])\n', s
 
             sio = StringIO.StringIO()
             sys.stdout = sio
             result = a("out", "in", env, presub=1)
-            assert result == 0, result
+            assert result == 7, result
+            s = sio.getvalue()
+            assert s == 'Building out with action:\n  execfunc(target, source, env)\nexecfunc(["out"], ["in"])\n', s
+
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = b(["out"], "in", env, presub=1)
+            assert result == 7, result
+            s = sio.getvalue()
+            assert s == 'Building out with action:\n  firstfunc(target, source, env)\nfirstfunc(["out"], ["in"])\nBuilding out with action:\n  execfunc(target, source, env)\nexecfunc(["out"], ["in"])\n', s
+
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = b(["out", "list"], "in", env, presub=1)
+            assert result == 7, result
             s = sio.getvalue()
-            assert s == 'Building out with action(s):\n  execfunc(env, target, source)\nexecfunc(["out"], ["in"])\n', s
+            assert s == 'Building out and list with action:\n  firstfunc(target, source, env)\nfirstfunc(["out", "list"], ["in"])\nBuilding out and list with action:\n  execfunc(target, source, env)\nexecfunc(["out", "list"], ["in"])\n', s
 
             a2 = SCons.Action.Action(execfunc)
 
             sio = StringIO.StringIO()
             sys.stdout = sio
             result = a2("out", "in", env)
-            assert result == 0, result
+            assert result == 7, result
             s = sio.getvalue()
-            assert s == 'Building out with action(s):\n  execfunc(env, target, source)\nexecfunc(["out"], ["in"])\n', s
+            assert s == 'Building out with action:\n  execfunc(target, source, env)\nexecfunc(["out"], ["in"])\n', s
+
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = a2("out", "in", env, presub=0)
+            assert result == 7, result
+            s = sio.getvalue()
+            assert s == 'execfunc(["out"], ["in"])\n', s
+
+            SCons.Action.execute_actions = 0
 
             sio = StringIO.StringIO()
             sys.stdout = sio
@@ -491,6 +544,8 @@ class ActionBaseTestCase(unittest.TestCase):
             assert result == 7, result
             assert errfunc_result == [7], errfunc_result
 
+            SCons.Action.execute_actions = 1
+
             result = []
             def my_print_cmd_line(s, target, source, env, result=result):
                 result.append(s)
@@ -521,7 +576,7 @@ class ActionBaseTestCase(unittest.TestCase):
             pass
         a = SCons.Action.Action(func)
         s = a.presub_lines(env)
-        assert s == ["func(env, target, source)"], s
+        assert s == ["func(target, source, env)"], s
 
         def gen(target, source, env, for_signature):
             return 'generat' + env.get('GEN', 'or')
@@ -634,7 +689,7 @@ class CommandActionTestCase(unittest.TestCase):
                                           '$TARGET', '$SOURCE',
                                           '$TARGETS', '$SOURCES'])
         s = str(act)
-        assert s == "['xyzzy', '$TARGET', '$SOURCE', '$TARGETS', '$SOURCES']", s
+        assert s == "xyzzy $TARGET $SOURCE $TARGETS $SOURCES", s
 
     def test_genstring(self):
         """Test the genstring() method for command Actions
@@ -666,7 +721,7 @@ class CommandActionTestCase(unittest.TestCase):
         act = SCons.Action.CommandAction(['xyzzy',
                                           '$TARGET', '$SOURCE',
                                           '$TARGETS', '$SOURCES'])
-        expect = "['xyzzy', '$TARGET', '$SOURCE', '$TARGETS', '$SOURCES']"
+        expect = "xyzzy $TARGET $SOURCE $TARGETS $SOURCES"
         s = act.genstring([], [], env)
         assert s == expect, s
         s = act.genstring([t1], [s1], env)
@@ -1105,32 +1160,6 @@ class CommandGeneratorActionTestCase(unittest.TestCase):
         assert self.dummy == 1, self.dummy
         assert s == "$FOO $TARGET $SOURCE $TARGETS $SOURCES", s
 
-    def test_strfunction(self):
-        """Test the command generator Action string function
-        """
-        def f(target, source, env, for_signature, self=self):
-            dummy = env['dummy']
-            self.dummy = dummy
-            return "$FOO"
-        a = SCons.Action.CommandGeneratorAction(f)
-        self.dummy = 0
-        s = a.strfunction([], [], env=Environment(FOO='xyzzy', dummy=1))
-        assert self.dummy == 1, self.dummy
-        assert s == 'xyzzy', s
-
-        def sf(target, source, env):
-            return "sf was called"
-        a = SCons.Action.CommandGeneratorAction(f, strfunction=sf)
-        s = a.strfunction([], [], env=Environment())
-        assert s == "sf was called", s
-
-        def f(target, source, env, for_signature, self=self):
-            def null(target, source, env):
-                pass
-            return SCons.Action.Action(null, strfunction=None)
-        a = SCons.Action.CommandGeneratorAction(f)
-        s = a.strfunction([], [], env=Environment())
-
     def test_execute(self):
         """Test executing a command generator Action
         """
@@ -1236,14 +1265,14 @@ class FunctionActionTestCase(unittest.TestCase):
             pass
         a = SCons.Action.FunctionAction(func1)
         s = str(a)
-        assert s == "func1(env, target, source)", s
+        assert s == "func1(target, source, env)", s
 
         class class1:
             def __call__(self):
                 pass
         a = SCons.Action.FunctionAction(class1())
         s = str(a)
-        assert s == "class1(env, target, source)", s
+        assert s == "class1(target, source, env)", s
 
     def test_execute(self):
         """Test executing a function Action
@@ -1372,7 +1401,7 @@ class ListActionTestCase(unittest.TestCase):
             pass
         a = SCons.Action.ListAction([f, g, "XXX", f])
         s = str(a)
-        assert s == "f(env, target, source)\ng(env, target, source)\nXXX\nf(env, target, source)", s
+        assert s == "f(target, source, env)\ng(target, source, env)\nXXX\nf(target, source, env)", s
 
     def test_genstring(self):
         """Test the genstring() method for a list of subsidiary Actions
@@ -1383,24 +1412,7 @@ class ListActionTestCase(unittest.TestCase):
             pass
         a = SCons.Action.ListAction([f, g, "XXX", f])
         s = a.genstring([], [], Environment())
-        assert s == "f(env, target, source)\ng(env, target, source)\nXXX\nf(env, target, source)", s
-
-    def test_strfunction(self):
-        """Test the string function for a list of subsidiary Actions
-        """
-        def f(target,source,env):
-            pass
-        def g(target,source,env):
-            pass
-        a = SCons.Action.ListAction([f, g, "XXX", f])
-        s = a.strfunction([], [], Environment())
-        assert s == "f([], [])\ng([], [])\nXXX\nf([], [])", s
-
-        def sf(target, source, env):
-            return "sf was called"
-        act = SCons.Action.ListAction([f, g, "XXX", f], strfunction=sf)
-        s = act.strfunction([], [], Environment())
-        assert s == "sf was called", s
+        assert s == "f(target, source, env)\ng(target, source, env)\nXXX\nf(target, source, env)", s
 
     def test_execute(self):
         """Test executing a list of subsidiary Actions
@@ -1466,15 +1478,6 @@ class LazyActionTestCase(unittest.TestCase):
         assert isinstance(a9, SCons.Action.CommandGeneratorAction), a10
         assert a10.generator.var == 'FOO', a10.generator.var
 
-    def test_strfunction(self):
-        """Test the lazy-evaluation Action string function
-        """
-        def f(target, source, env):
-            pass
-        a = SCons.Action.Action('$BAR')
-        s = a.strfunction([], [], env=Environment(BAR=f, s=self))
-        assert s == "f([], [])", s
-
     def test_genstring(self):
         """Test the lazy-evaluation Action genstring() method
         """
@@ -1482,7 +1485,7 @@ class LazyActionTestCase(unittest.TestCase):
             pass
         a = SCons.Action.Action('$BAR')
         s = a.genstring([], [], env=Environment(BAR=f, s=self))
-        assert s == "f(env, target, source)", s
+        assert s == "f(target, source, env)", s
 
     def test_execute(self):
         """Test executing a lazy-evaluation Action
@@ -1607,7 +1610,12 @@ class ActionFactoryTestCase(unittest.TestCase):
         af = SCons.Action.ActionFactory(actfunc, strfunc)
         af(3, 6, 9)([], [], Environment())
         assert actfunc_args == [3, 6, 9], actfunc_args
-        assert strfunc_args == [3, 6, 9], strfunc_args
+        # Note that strfunc gets evaluated twice: once when we called
+        # the actionfactory itself to get the real action
+        # (Action(ActionCaller, ...)), and once when we actually call
+        # that resulting action; since strfunc modifies the global,
+        # account for the number of times it was called.
+        assert strfunc_args == [3, 6, 9, 3, 6, 9], strfunc_args
 
 
 class ActionCompareTestCase(unittest.TestCase):
@@ -1664,6 +1672,7 @@ if __name__ == "__main__":
     suite = unittest.TestSuite()
     tclasses = [ ActionTestCase,
                  ActionBaseTestCase,
+                 _ActionActionTestCase,
                  CommandActionTestCase,
                  CommandGeneratorActionTestCase,
                  FunctionActionTestCase,
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 6afff58..d6b7597 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -291,7 +291,7 @@ def _init_nodes(builder, env, overrides, executor_kw, tlist, slist):
 
                 if t_contents == contents:
                     SCons.Warnings.warn(SCons.Warnings.DuplicateEnvironmentWarning,
-                                        "Two different environments were specified for target %s,\n\tbut they appear to have the same action: %s"%(str(t), t.builder.action.strfunction(tlist, slist, t.env)))
+                                        "Two different environments were specified for target %s,\n\tbut they appear to have the same action: %s"%(str(t), t.builder.action.genstring(tlist, slist, t.env)))
 
                 else:
                     raise UserError, "Two environments with different actions were specified for the same target: %s"%str(t)
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index 68af6ed..6a8fe83 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -88,36 +88,15 @@ class Executor:
                                                          self.sources))
             return self.build_env
 
-    def get_action_list(self, target):
-        """Fetch or create the appropriate action list (for this target).
-
-        There is an architectural mistake here: we cache the action list
-        for the Executor and re-use it regardless of which target is
-        being asked for.  In practice, this doesn't seem to be a problem
-        because executing the action list will update all of the targets
-        involved, so only one target's pre- and post-actions will win,
-        anyway.  This is probably a bug we should fix...
-        """
-        al = [self.action]
-        try:
-            # XXX shouldn't reach into node attributes like this
-            return target.pre_actions + al + target.post_actions
-        except AttributeError:
-            return al
-
     def do_nothing(self, target, errfunc, **kw):
         pass
 
     def __call__(self, target, errfunc, **kw):
         """Actually execute the action list."""
-        action_list = self.get_action_list(target)
-        if not action_list:
-            return
-        env = self.get_build_env()
         kw = kw.copy()
         kw.update(self.builder_kw)
-        for action in action_list:
-            apply(action, (self.targets, self.sources, env, errfunc), kw)
+        apply(self.action, (self.targets, self.sources,
+                            self.get_build_env(), errfunc), kw)
 
     def cleanup(self):
         try:
@@ -142,45 +121,9 @@ class Executor:
                                            self.get_build_env())
             return self.string
 
-    def strfunction(self):
-        try:
-            return self._strfunc
-        except AttributeError:
-            action = self.action
-            build_env = self.get_build_env()
-            if action.strfunction is None:
-                # This instance has strfunction set to None to suppress
-                # printing of the action.  Call the method directly
-                # through the class instead.
-                self._strfunc = action.__class__.strfunction(action,
-                                                             self.targets,
-                                                             self.sources,
-                                                             build_env)
-            else:
-                self._strfunc = action.strfunction(self.targets,
-                                                   self.sources,
-                                                   build_env)
-            return self._strfunc
-
     def nullify(self):
         self.__call__ = self.do_nothing
         self.string = ''
-        self._strfunc = None
-
-    def get_raw_contents(self):
-        """Fetch the raw signature contents.  This, along with
-        get_contents(), is the real reason this class exists, so we can
-        compute this once and cache it regardless of how many target or
-        source Nodes there are.
-        """
-        try:
-            return self.raw_contents
-        except AttributeError:
-            action = self.action
-            self.raw_contents = action.get_raw_contents(self.targets,
-                                                        self.sources,
-                                                        self.get_build_env())
-            return self.raw_contents
 
     def get_contents(self):
         """Fetch the signature contents.  This, along with
diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py
index dc98818..219efee 100644
--- a/src/engine/SCons/ExecutorTests.py
+++ b/src/engine/SCons/ExecutorTests.py
@@ -48,13 +48,9 @@ class MyAction:
         self.actions = actions
     def __call__(self, target, source, env, errfunc, **kw):
         for action in self.actions:
-            action(target, source, env, errfunc)
-    def strfunction(self, target, source, env):
-        return string.join(['STRFUNCTION'] + map(str, self.actions) + target + source)
+            apply(action, (target, source, env, errfunc), kw)
     def genstring(self, target, source, env):
         return string.join(['GENSTRING'] + map(str, self.actions) + target + source)
-    def get_raw_contents(self, target, source, env):
-        return string.join(['RAW'] + self.actions + target + source)
     def get_contents(self, target, source, env):
         return string.join(self.actions + target + source)
 
@@ -68,6 +64,15 @@ class MyNode:
     def __init__(self, pre, post):
         self.pre_actions = pre
         self.post_actions = post
+    def build(self, errfunc=None):
+        executor = SCons.Executor.Executor(MyAction(self.pre_actions +
+                                                    [self.builder.action] +
+                                                    self.post_actions),
+                                           self.builder.env,
+                                           [],
+                                           [self],
+                                           ['s1', 's2'])
+        apply(executor, (self, errfunc), {})
         
 
 class ExecutorTestCase(unittest.TestCase):
@@ -118,19 +123,6 @@ class ExecutorTestCase(unittest.TestCase):
         assert be['O'] == 'ob3', be['O']
         assert be['Y'] == 'yyy', be['Y']
 
-    def test_get_action_list(self):
-        """Test fetching and generating an action list"""
-        x = SCons.Executor.Executor('b', 'e', 'o', 't', 's')
-        al = x.get_action_list(MyNode([], []))
-        assert al == ['b'], al
-        al = x.get_action_list(MyNode(['PRE'], ['POST']))
-        assert al == ['PRE', 'b', 'POST'], al
-
-        a = MyAction()
-        x = SCons.Executor.Executor(a, None, {}, 't', 's')
-        al = x.get_action_list(MyNode(['pre'], ['post']))
-        assert al == ['pre', a, 'post'], al
-
     def test__call__(self):
         """Test calling an Executor"""
         result = []
@@ -145,9 +137,11 @@ class ExecutorTestCase(unittest.TestCase):
 
         env = MyEnvironment()
         a = MyAction([action1, action2])
-        x = SCons.Executor.Executor(a, env, [], ['t1', 't2'], ['s1', 's2'])
-
-        x(MyNode([pre], [post]), None)
+        b = MyBuilder(env, {})
+        b.action = a
+        n = MyNode([pre], [post])
+        n.builder = b
+        n.build()
         assert result == ['pre', 'action1', 'action2', 'post'], result
         del result[:]
 
@@ -157,7 +151,9 @@ class ExecutorTestCase(unittest.TestCase):
                 errfunc(1)
             return 1
 
-        x(MyNode([pre_err], [post]), None)
+        n = MyNode([pre_err], [post])
+        n.builder = b
+        n.build()
         assert result == ['pre_err', 'action1', 'action2', 'post'], result
         del result[:]
 
@@ -165,7 +161,7 @@ class ExecutorTestCase(unittest.TestCase):
             raise "errfunc %s" % stat
 
         try:
-            x(MyNode([pre_err], [post]), errfunc)
+            n.build(errfunc)
         except:
             assert sys.exc_type == "errfunc 1", sys.exc_type
         else:
@@ -204,14 +200,6 @@ class ExecutorTestCase(unittest.TestCase):
         c = str(x)
         assert c == 'GENSTRING action1 action2 t s', c
 
-    def test_strfunction(self):
-        """Test the strfunction() method"""
-        env = MyEnvironment(S='string')
-
-        x = SCons.Executor.Executor(MyAction(), env, [], ['t'], ['s'])
-        s = x.strfunction()
-        assert s == 'STRFUNCTION action1 action2 t s', s
-
     def test_nullify(self):
         """Test the nullify() method"""
         env = MyEnvironment(S='string')
@@ -228,8 +216,6 @@ class ExecutorTestCase(unittest.TestCase):
         assert result == ['action1'], result
         s = str(x)
         assert s[:10] == 'GENSTRING ', s
-        s = x.strfunction()
-        assert s[:12] == 'STRFUNCTION ', s
 
         del result[:]
         x.nullify()
@@ -238,21 +224,6 @@ class ExecutorTestCase(unittest.TestCase):
         assert result == [], result
         s = str(x)
         assert s == '', s
-        s = x.strfunction()
-        assert s == None, s
-
-    def test_get_raw_contents(self):
-        """Test fetching the raw signatures contents"""
-        env = MyEnvironment(RC='raw contents')
-
-        x = SCons.Executor.Executor(MyAction(), env, [], ['t'], ['s'])
-        x.raw_contents = 'raw raw raw'
-        rc = x.get_raw_contents()
-        assert rc == 'raw raw raw', rc
-
-        x = SCons.Executor.Executor(MyAction(), env, [], ['t'], ['s'])
-        rc = x.get_raw_contents()
-        assert rc == 'RAW action1 action2 t s', rc
 
     def test_get_contents(self):
         """Test fetching the signatures contents"""
@@ -267,7 +238,7 @@ class ExecutorTestCase(unittest.TestCase):
         c = x.get_contents()
         assert c == 'action1 action2 t s', c
 
-    def test_get_timetstamp(self):
+    def test_get_timestamp(self):
         """Test fetching the "timestamp" """
         x = SCons.Executor.Executor('b', 'e', 'o', 't', ['s1', 's2'])
         ts = x.get_timestamp()
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index ce67781..508b70e 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -39,7 +39,27 @@ built_source =  None
 cycle_detected = None
 built_order = 0
 
-class MyAction:
+def _actionAppend(a1, a2):
+    all = []
+    for curr_a in [a1, a2]:
+        if isinstance(curr_a, MyAction):
+            all.append(curr_a)
+        elif isinstance(curr_a, MyListAction):
+            all.extend(curr_a.list)
+        elif type(curr_a) == type([1,2]):
+            all.extend(curr_a)
+        else:
+            raise 'Cannot Combine Actions'
+    return MyListAction(all)
+
+class MyActionBase:
+    def __add__(self, other):
+        return _actionAppend(self, other)
+
+    def __radd__(self, other):
+        return _actionAppend(other, self)
+
+class MyAction(MyActionBase):
     def __init__(self):
         self.order = 0
 
@@ -53,10 +73,14 @@ class MyAction:
         self.order = built_order
         return 0
 
-    def get_actions(self):
-        return [self]
-
-class MyNonGlobalAction:
+class MyListAction(MyActionBase):
+    def __init__(self, list):
+        self.list = list
+    def __call__(self, target, source, env, errfunc):
+        for A in self.list:
+            A(target, source, env, errfunc)
+        
+class MyNonGlobalAction(MyActionBase):
     def __init__(self):
         self.order = 0
         self.built_it = None
@@ -74,9 +98,6 @@ class MyNonGlobalAction:
         self.order = built_order
         return 0
 
-    def get_actions(self):
-        return [self]
-
 class Environment:
     def __init__(self, **kw):
         self._dict = {}
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 38cff92..31f8b58 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -167,7 +167,12 @@ class Node:
             if not create:
                 raise
             import SCons.Executor
-            executor = SCons.Executor.Executor(self.builder.action,
+            act = self.builder.action
+            if self.pre_actions:
+                act = self.pre_actions + act
+            if self.post_actions:
+                act = act + self.post_actions
+            executor = SCons.Executor.Executor(act,
                                                self.builder.env,
                                                [self.builder.overrides],
                                                [self],
@@ -175,6 +180,13 @@ class Node:
             self.executor = executor
         return executor
 
+    def reset_executor(self):
+        "Remove cached executor; forces recompute when needed."
+        try:
+            delattr(self, 'executor')
+        except AttributeError:
+            pass
+
     def retrieve_from_cache(self):
         """Try to retrieve the node's content from a cache
 
@@ -565,7 +577,7 @@ class Node:
 
         if self.has_builder():
             executor = self.get_executor()
-            binfo.bact = executor.strfunction()
+            binfo.bact = str(executor)
             binfo.bactsig = calc.module.signature(executor)
             sigs.append(binfo.bactsig)
 
@@ -795,11 +807,15 @@ class Node:
         """Adds an Action performed on this Node only before
         building it."""
         self.pre_actions.append(act)
+        # executor must be recomputed to include new pre-actions
+        self.reset_executor()
 
     def add_post_action(self, act):
         """Adds and Action performed on this Node only after
         building it."""
         self.post_actions.append(act)
+        # executor must be recomputed to include new pre-actions
+        self.reset_executor()
 
     def render_include_tree(self):
         """