Refactor action execution so it's controlled by the interface-specific Taskmaster.Task class, not Node.build().

11 files changed, 158 insertions, 27 deletions

diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 534b813..8732bd7 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -129,6 +129,9 @@ class ActionBase:
     def show(self, string):
         print string
 
+    def get_actions(self):
+        return [self]
+
     def subst_dict(self, target, source, env):
         """Create a dictionary for substitution of construction
         variables.
@@ -191,6 +194,14 @@ class CommandAction(ActionBase):
         self.cmd_list = cmd
 
     def execute(self, target, source, env):
+        """Execute a command action.
+
+        This will handle lists of commands as well as individual commands,
+        because construction variable substitution may turn a single
+        "command" into a list.  This means that this class can actually
+        handle lists of commands, even though that's not how we use it
+        externally.
+        """
         escape = env.get('ESCAPE', lambda x: x)
 
         import SCons.Errors
@@ -206,7 +217,6 @@ class CommandAction(ActionBase):
             raise SCons.Errors.UserError('Missing SPAWN construction variable.')
 
         dict = self.subst_dict(target, source, env)
-        import SCons.Util
         cmd_list = SCons.Util.scons_subst_list(self.cmd_list, dict, {}, _rm)
         for cmd_line in cmd_list:
             if len(cmd_line):
@@ -342,6 +352,9 @@ class ListAction(ActionBase):
     def __init__(self, list):
         self.list = map(lambda x: Action(x), list)
 
+    def get_actions(self):
+        return self.list
+
     def execute(self, target, source, env):
         for l in self.list:
             r = l.execute(target, source, env)
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 86b84b9..dfa41ad 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -46,7 +46,7 @@ def Environment(dict):
 
 class ActionTestCase(unittest.TestCase):
 
-    def runTest(self):
+    def test_factory(self):
         """Test the Action factory
         """
         def foo():
@@ -56,6 +56,7 @@ class ActionTestCase(unittest.TestCase):
 
         a2 = SCons.Action.Action("string")
         assert isinstance(a2, SCons.Action.CommandAction), a2
+        assert a2.cmd_list == ["string"], a2.cmd_list
 
         if hasattr(types, 'UnicodeType'):
             exec "a3 = SCons.Action.Action(u'string')"
@@ -64,8 +65,11 @@ class ActionTestCase(unittest.TestCase):
         a4 = SCons.Action.Action(["x", "y", "z", [ "a", "b", "c"]])
         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 isinstance(a4.list[1], SCons.Action.CommandAction), a4.list[1]
+        assert a4.list[1].cmd_list == ["y"], a4.list[1].cmd_list
         assert isinstance(a4.list[2], SCons.Action.CommandAction), a4.list[2]
+        assert a4.list[2].cmd_list == ["z"], a4.list[2].cmd_list
         assert isinstance(a4.list[3], SCons.Action.CommandAction), a4.list[3]
         assert a4.list[3].cmd_list == [ "a", "b", "c" ], a4.list[3].cmd_list
 
@@ -83,6 +87,15 @@ class ActionTestCase(unittest.TestCase):
         assert isinstance(a8, SCons.Action.CommandAction), a8
         assert a8.cmd_list == [ "a8" ], a8.cmd_list
 
+        a9 = SCons.Action.Action("x\ny\nz")
+        assert isinstance(a9, SCons.Action.ListAction), a9
+        assert isinstance(a9.list[0], SCons.Action.CommandAction), a9.list[0]
+        assert a9.list[0].cmd_list == ["x"], a9.list[0].cmd_list
+        assert isinstance(a9.list[1], SCons.Action.CommandAction), a9.list[1]
+        assert a9.list[1].cmd_list == ["y"], a9.list[1].cmd_list
+        assert isinstance(a9.list[2], SCons.Action.CommandAction), a9.list[2]
+        assert a9.list[2].cmd_list == ["z"], a9.list[2].cmd_list
+
 class ActionBaseTestCase(unittest.TestCase):
 
     def test_cmp(self):
@@ -95,6 +108,13 @@ class ActionBaseTestCase(unittest.TestCase):
         assert a1 != a3
         assert a2 != a3
 
+    def test_get_actions(self):
+        """Test the get_actions() method
+        """
+        a = SCons.Action.Action("x")
+        l = a.get_actions()
+        assert l == [a], l
+
     def test_subst_dict(self):
         """Test substituting dictionary values in an Action
         """
@@ -110,7 +130,6 @@ class ActionBaseTestCase(unittest.TestCase):
         assert str(d['SOURCES']) == 's', d['SOURCES']
         assert str(d['SOURCE']) == 's', d['SOURCE']
 
-
         d = a.subst_dict(target = ['t1', 't2'], source = ['s1', 's2'], env=Environment({}))
         TARGETS = map(lambda x: str(x), d['TARGETS'])
         TARGETS.sort()
@@ -198,9 +217,9 @@ class CommandActionTestCase(unittest.TestCase):
         """
         a = SCons.Action.CommandAction(["|", "$(", "$foo", "|", "$bar",
                                         "$)", "|"])
-        c = a.get_contents(target=[], source=[],
-                           foo = 'FFF', bar = 'BBB')
-        assert c == "| $( FFF | BBB $) |"
+        c = a.get_raw_contents(target=[], source=[],
+                               env=Environment({'foo':'FFF', 'bar':'BBB'}))
+        assert c == "| $( FFF | BBB $) |", c
 
     def test_get_contents(self):
         """Test fetching the contents of a command Action
@@ -348,6 +367,19 @@ class ListActionTestCase(unittest.TestCase):
         assert isinstance(a.list[2], SCons.Action.ListAction)
         assert a.list[2].list[0].cmd_list == [ 'y' ]
 
+    def test_get_actions(self):
+        """Test the get_actions() method for ListActions
+        """
+        a = SCons.Action.ListAction(["x", "y"])
+        l = a.get_actions()
+        assert len(l) == 2, l
+        assert isinstance(l[0], SCons.Action.CommandAction), l[0]
+        g = l[0].get_actions()
+        assert g == [l[0]], g
+        assert isinstance(l[1], SCons.Action.CommandAction), l[1]
+        g = l[1].get_actions()
+        assert g == [l[1]], g
+
     def test_execute(self):
         """Test executing a list of subsidiary Actions
         """
@@ -390,7 +422,7 @@ class LazyActionTestCase(unittest.TestCase):
         assert a10.generator.var == 'FOO', a10.generator.var
 
     def test_execute(self):
-        """Test executing a lazy-evalueation Action
+        """Test executing a lazy-evaluation Action
         """
         def f(target, source, env):
             s = env['s']
@@ -411,15 +443,15 @@ class LazyActionTestCase(unittest.TestCase):
 
 if __name__ == "__main__":
     suite = unittest.TestSuite()
-    suite.addTest(ActionTestCase())
-    suite.addTest(ActionBaseTestCase("test_cmp"))
-    suite.addTest(ActionBaseTestCase("test_subst_dict"))
-    for tclass in [CommandActionTestCase,
-                   CommandGeneratorActionTestCase,
-                   FunctionActionTestCase,
-                   ListActionTestCase,
-                   LazyActionTestCase]:
-        for func in ["test_init", "test_execute", "test_get_contents"]:
-            suite.addTest(tclass(func))
+    tclasses = [ ActionTestCase,
+                 ActionBaseTestCase,
+                 CommandActionTestCase,
+                 CommandGeneratorActionTestCase,
+                 FunctionActionTestCase,
+                 ListActionTestCase,
+                 LazyActionTestCase]
+    for tclass in tclasses:
+        names = unittest.getTestCaseNames(tclass, 'test_')
+        suite.addTests(map(tclass, names))
     if not unittest.TextTestRunner().run(suite).wasSuccessful():
         sys.exit(1)
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 02f9e2d..08d8f0c 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -317,6 +317,9 @@ class BuilderBase:
 
         return tlist
 
+    def get_actions(self):
+        return self.action.get_actions()
+
     def execute(self, target, source, env):
         """Execute a builder's action to create an output object.
         """
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index b087d7b..3c81d38 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -406,6 +406,18 @@ class BuilderTestCase(unittest.TestCase):
         r = builder.execute([],[],Environment(out = outfile))
         assert r == expect_nonexecutable, "r == %d" % r
 
+    def test_get_actions(self):
+        """Test fetching the Builder's Action list
+
+        Verify that we call the underlying Action's method
+        """
+        builder = SCons.Builder.Builder(name="builder", action=SCons.Action.ListAction(["x", "y", "z"]))
+        a = builder.get_actions()
+        assert len(a) == 3, a
+        assert isinstance(a[0], SCons.Action.CommandAction), a[0]
+        assert isinstance(a[1], SCons.Action.CommandAction), a[1]
+        assert isinstance(a[2], SCons.Action.CommandAction), a[2]
+
     def test_get_contents(self):
         """Test returning the signature contents of a Builder
         """
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index a3ac7be..4ed4664 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -659,6 +659,10 @@ class Dir(Entry):
         kids.sort(c)
         return kids + SCons.Node.Node.all_children(self, 0)
 
+    def get_actions(self):
+        """A null "builder" for directories."""
+        return []
+
     def build(self):
         """A null "builder" for directories."""
         pass
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index 8386f83..5917044 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -944,6 +944,15 @@ class prepareTestCase(unittest.TestCase):
             exc_caught = 1
         assert exc_caught, "Should have caught a StopError."
 
+class get_actionsTestCase(unittest.TestCase):
+    def runTest(self):
+        """Test the Dir's get_action() method"""
+
+        fs = SCons.Node.FS.FS()
+        dir = fs.Dir('.')
+        a = dir.get_actions()
+        assert a == [], a
+
 
 if __name__ == "__main__":
     suite = unittest.TestSuite()
@@ -953,5 +962,6 @@ if __name__ == "__main__":
     suite.addTest(find_fileTestCase())
     suite.addTest(StringDirTestCase())
     suite.addTest(prepareTestCase())
+    suite.addTest(get_actionsTestCase())
     if not unittest.TextTestRunner().run(suite).wasSuccessful():
         sys.exit(1)
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index cd2d2e3..4054bb9 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -46,6 +46,8 @@ class Builder:
         built_source = source
         built_args = env
         return 0
+    def get_actions(self):
+        return 'xyzzy'
     def get_contents(self, target, source, env):
         return 7
 
@@ -242,6 +244,14 @@ class NodeTestCase(unittest.TestCase):
         node.env_set(e)
         assert node.env == e
 
+    def test_get_actions(self):
+        """Test fetching a Node's action list
+        """
+        node = SCons.Node.Node()
+        node.builder_set(Builder())
+        a = node.get_actions()
+        assert a == 'xyzzy', a
+
     def test_set_bsig(self):
         """Test setting a Node's signature
         """
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 2033663..82dfd7e 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -110,6 +110,10 @@ class Node:
     def generate_build_env(self):
         return self.env.Override(self.overrides)
 
+    def get_actions(self):
+        """Fetch the action list to build."""
+        return self.builder.get_actions()
+
     def build(self):
         """Actually build the node.   Return the status from the build."""
         # This method is called from multiple threads in a parallel build,
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index ad1a641..add52e4 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -73,14 +73,34 @@ from SCons.Optik import OptionParser, SUPPRESS_HELP, OptionValueError
 class BuildTask(SCons.Taskmaster.Task):
     """An SCons build task."""
     def execute(self):
-        t = self.targets[0]
-        if t.get_state() == SCons.Node.up_to_date:
-            if self.top and t.builder:
-                display('scons: "%s" is up to date.' % str(self.targets[0]))
-        else:
+        target = self.targets[0]
+        if target.get_state() == SCons.Node.up_to_date:
+            if self.top and target.builder:
+                display('scons: "%s" is up to date.' % str(target))
+        elif target.builder and not hasattr(target.builder, 'status'):
+            action_list = target.get_actions()
+            if not action_list:
+                return
+            env = target.generate_build_env()
             if print_time:
                 start_time = time.time()
-            self.targets[0].build()
+            try:
+                for action in action_list:
+                    stat = action.execute(self.targets, target.sources, env)
+                    if stat:
+                        raise BuildError(node = target,
+                                         errstr = "Error %d" % stat)
+            except KeyboardInterrupt:
+                raise
+            except UserError:
+                raise
+            except BuildError:
+                raise
+            except:
+                raise BuildError(target, "Exception",
+                                 sys.exc_type,
+                                 sys.exc_value,
+                                 sys.exc_traceback)
             if print_time:
                 finish_time = time.time()
                 global command_time
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index a32def4..043ce7d 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -60,11 +60,14 @@ class Task:
         self.top = top
         self.node = node
 
-
     def prepare(self):
-        """Called just before the task is executed."""
+        """Called just before the task is executed.
+
+        This unlinks all targets and makes all directories before
+        building anything."""
         if self.targets[0].get_state() != SCons.Node.up_to_date:
-            self.targets[0].prepare()
+            for t in self.targets:
+                t.prepare()
 
     def execute(self):
         """Called to execute the task.
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 23d7e0a..c89f356 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -45,6 +45,7 @@ class Node:
         self.bsig = None
         self.csig = None
         self.state = None
+        self.prepared = None
         self.parents = []
         self.side_effect = 0
         self.side_effects = []
@@ -61,7 +62,7 @@ class Node:
         built_text = built_text + " really"
 
     def prepare(self):
-        pass
+        self.prepared = 1
 
     def children(self):
         if not self.scanned:
@@ -410,6 +411,25 @@ class TaskmasterTestCase(unittest.TestCase):
     def test_executed(self):
         pass
 
+    def test_prepare(self):
+        """Test preparation of multiple Nodes for a task
+
+        """
+        n1 = Node("n1")
+        n2 = Node("n2")
+        tm = SCons.Taskmaster.Taskmaster([n1, n2])
+        t = tm.next_task()
+        # This next line is moderately bogus.  We're just reaching
+        # in and setting the targets for this task to an array.  The
+        # "right" way to do this would be to have the next_task() call
+        # set it up by having something that approximates a real Builder
+        # return this list--but that's more work than is probably
+        # warranted right now.
+        t.targets = [n1, n2]
+        t.prepare()
+        assert n1.prepared
+        assert n2.prepared
+
 
 
 if __name__ == "__main__":