Officially support target_factory and source_factory when creating a Builder.

11 files changed, 291 insertions, 88 deletions

diff --git a/doc/man/scons.1 b/doc/man/scons.1
index 63c6a83..947804f 100644
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -6288,8 +6288,25 @@ opts.AddOptions(
 .SH EXTENDING SCONS
 .SS Builder Objects
 .B scons
-can be extended by adding new builders to a construction
-environment using the 
+can be extended to build different types of targets
+by adding new Builder objects
+to a construction environment.
+.IR "In general" ,
+you should only need to add a new Builder object
+when you want to build a new type of file or other external target.
+If you just want to invoke a different compiler or other tool
+to build a Program, Object, Library, or any other
+type of output file for which
+.B scons
+already has an existing Builder,
+it is generally much easier to
+use those existing Builders
+in a construction environment
+that sets the appropriate construction variables
+(CC, LINK, etc.).
+
+Builder objects are created
+using the 
 .B Builder 
 function.
 The
@@ -6391,12 +6408,58 @@ lines in source files.
 (See the section "Scanner Objects," below,
 for information about creating Scanner objects.)
 
+.IP target_factory
+A factory function that the Builder will use
+to turn any targets specified as strings into SCons Nodes.
+By default,
+SCons assumes that all targets are files.
+Other useful target_factory
+values include
+.BR Dir ,
+for when a Builder creates a directory target,
+and
+.BR Entry ,
+for when a Builder can create either a file
+or directory target.
+
+Example:
+
+.ES
+MakeDirectoryBuilder = Builder(action=my_mkdir, target_factory=Dir)
+env = Environment()
+env.Append(BUILDERS = {'MakeDirectory':MakeDirectoryBuilder})
+env.MakeDirectory('new_directory')
+.EE
+
+.IP source_factory
+A factory function that the Builder will use
+to turn any sources specified as strings into SCons Nodes.
+By default,
+SCons assumes that all source are files.
+Other useful source_factory
+values include
+.BR Dir ,
+for when a Builder uses a directory as a source,
+and
+.BR Entry ,
+for when a Builder can use files
+or directories (or both) as sources.
+
+Example:
+
+.ES
+CollectBuilder = Builder(action=my_mkdir, source_factory=Entry)
+env = Environment()
+env.Append(BUILDERS = {'Collect':CollectBuilder})
+env.Collect('archive', ['directory_name', 'file_name'])
+.EE
+
 .IP emitter
 A function or list of functions to manipulate the target and source
 lists before dependencies are established
 and the target(s) are actually built.
 .B emitter
-can also be string containing a construction variable to expand
+can also be a string containing a construction variable to expand
 to an emitter function or list of functions,
 or a dictionary mapping source file suffixes
 to emitter functions.
@@ -6453,6 +6516,12 @@ b = Builder("my_build < $TARGET > $SOURCE",
             emitter = {'.suf1' : e_suf1,
                        '.suf2' : e_suf2})
 .EE
+.IP
+The 
+.I generator
+and
+.I action
+arguments must not both be used for the same Builder.
 
 .IP multi
 Specifies whether this builder is allowed to be called multiple times for
@@ -6508,12 +6577,29 @@ any of the suffixes of the builder. Using this argument produces a
 multi-stage builder.
 
 .RE
+.IP
 The 
 .I generator
 and
 .I action
 arguments must not both be used for the same Builder.
 
+.IP env
+A construction environment that can be used
+to fetch source code using this Builder.
+(Note that this environment is
+.I not
+used for normal builds of normal target files,
+which use the environment that was
+used to call the Builder for the target file.)
+
+.ES
+b = Builder(action="build < $SOURCE > $TARGET")
+env = Environment(BUILDERS = {'MyBuild' : b})
+env.MyBuild('foo.out', 'foo.in', my_arg = 'xyzzy')
+.EE
+
+.RE
 Any additional keyword arguments supplied
 when a Builder object is created
 (that is, when the Builder() function is called)
@@ -6532,12 +6618,6 @@ created by that particular Builder call
 (and any other files built as a
 result of the call).
 
-.ES
-b = Builder(action="build < $SOURCE > $TARGET")
-env = Environment(BUILDERS = {'MyBuild' : b})
-env.MyBuild('foo.out', 'foo.in', my_arg = 'xyzzy')
-.EE
-
 These extra keyword arguments are passed to the
 following functions:
 command generator functions,
diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index ee75857..f2ee12d 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -138,6 +138,10 @@ RELEASE 0.96 - XXX
   - Slim down the internal Sig.Calculator class by eliminating methods
     whose functionality is now covered by Node methods.
 
+  - Document use of the target_factory and source_factory keyword
+    arguments when creating Builder objects.  Enhance Dir Nodes so that
+    they can be created with user-specified Builder objects.
+
   From Gary Oberbrunner:
 
   - Add a --debug=presub option to print actions prior to substitution.
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index c8d62cd..6a90c76 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -191,25 +191,29 @@ def _do_create_action(act, *args, **kw):
             return apply(ListAction, (listCmdActions,)+args, kw)
     return None
 
-def Action(act, strfunction=_null, varlist=[]):
+def Action(act, strfunction=_null, varlist=[], presub=_null):
     """A factory for action objects."""
     if SCons.Util.is_List(act):
-        acts = map(lambda x, s=strfunction, v=varlist:
-                          _do_create_action(x, strfunction=s, varlist=v),
+        acts = map(lambda x, s=strfunction, v=varlist, ps=presub:
+                          _do_create_action(x, strfunction=s, varlist=v, presub=ps),
                    act)
         acts = filter(lambda x: not x is None, acts)
         if len(acts) == 1:
             return acts[0]
         else:
-            return ListAction(acts, strfunction=strfunction, varlist=varlist)
+            return ListAction(acts, strfunction=strfunction, varlist=varlist, presub=presub)
     else:
-        return _do_create_action(act, strfunction=strfunction, varlist=varlist)
+        return _do_create_action(act, strfunction=strfunction, varlist=varlist, presub=presub)
 
 class ActionBase:
     """Base class for actions that create output objects."""
-    def __init__(self, strfunction=_null, **kw):
+    def __init__(self, strfunction=_null, presub=_null, **kw):
         if not strfunction is _null:
             self.strfunction = strfunction
+        if presub is _null:
+            self.presub = print_actions_presub
+        else:
+            self.presub = presub
 
     def __cmp__(self, other):
         return cmp(self.__dict__, other.__dict__)
@@ -223,12 +227,12 @@ class ActionBase:
             target = [target]
         if not SCons.Util.is_List(source):
             source = [source]
-        if presub is _null:  presub = print_actions_presub
+        if presub is _null:  presub = self.presub
         if show is _null:  show = print_actions
         if execute is _null:  execute = execute_actions
         if presub:
             t = string.join(map(str, target), 'and')
-            l = string.join(self.presub(env), '\n  ')
+            l = string.join(self.presub_lines(env), '\n  ')
             out = "Building %s with action(s):\n  %s\n" % (t, l)
             sys.stdout.write(out)
         if show and self.strfunction:
@@ -243,7 +247,7 @@ class ActionBase:
         else:
             return 0
 
-    def presub(self, env):
+    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
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index f699e61..9a89623 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -392,11 +392,27 @@ class ActionBaseTestCase(unittest.TestCase):
             result = a("out", "in", env)
             assert result == 0, 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
+            s = sio.getvalue()
+            assert s == 'Building out with action(s):\n  execfunc(env, target, source)\nexecfunc("out", "in")\n', s
+
+            a2 = SCons.Action.Action(execfunc)
+
+            sio = StringIO.StringIO()
+            sys.stdout = sio
+            result = a2("out", "in", env)
+            assert result == 0, result
+            s = sio.getvalue()
             assert s == 'Building out with action(s):\n  execfunc(env, target, source)\nexecfunc("out", "in")\n', s
 
             sio = StringIO.StringIO()
             sys.stdout = sio
-            result = a("out", "in", env, presub=0)
+            result = a2("out", "in", env, presub=0)
             assert result == 0, result
             s = sio.getvalue()
             assert s == 'execfunc("out", "in")\n', s
@@ -428,36 +444,36 @@ class ActionBaseTestCase(unittest.TestCase):
             SCons.Action.print_actions_presub = save_print_actions_presub
             SCons.Action.execute_actions = save_execute_actions
 
-    def test_presub(self):
-        """Test the presub() method
+    def test_presub_lines(self):
+        """Test the presub_lines() method
         """
         env = Environment()
         a = SCons.Action.Action("x")
-        s = a.presub(env)
+        s = a.presub_lines(env)
         assert s == ['x'], s
 
         a = SCons.Action.Action(["y", "z"])
-        s = a.presub(env)
+        s = a.presub_lines(env)
         assert s == ['y', 'z'], s
 
         def func():
             pass
         a = SCons.Action.Action(func)
-        s = a.presub(env)
+        s = a.presub_lines(env)
         assert s == ["func(env, target, source)"], s
 
         def gen(target, source, env, for_signature):
             return 'generat' + env.get('GEN', 'or')
         a = SCons.Action.Action(SCons.Action.CommandGenerator(gen))
-        s = a.presub(env)
+        s = a.presub_lines(env)
         assert s == ["generator"], s
-        s = a.presub(Environment(GEN = 'ed'))
+        s = a.presub_lines(Environment(GEN = 'ed'))
         assert s == ["generated"], s
 
         a = SCons.Action.Action("$ACT")
-        s = a.presub(env)
+        s = a.presub_lines(env)
         assert s == [''], s
-        s = a.presub(Environment(ACT = 'expanded action'))
+        s = a.presub_lines(Environment(ACT = 'expanded action'))
         assert s == ['expanded action'], s
 
     def test_get_actions(self):
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index 733a26a..1dcf84c 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -285,7 +285,7 @@ def _init_nodes(builder, env, overrides, tlist, slist):
         if t.side_effect:
             raise UserError, "Multiple ways to build the same target were specified for: %s" % str(t)
         if t.has_builder():
-            if not t.env is env:
+            if not t.env is None and not t.env is env:
                 t_contents = t.builder.action.get_contents(tlist, slist, t.env)
                 contents = t.builder.action.get_contents(tlist, slist, env)
 
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 5766523..080232a 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -1172,10 +1172,7 @@ class Base:
         targets = self.arg2nodes(target, self.fs.Entry)
 
         for side_effect in side_effects:
-            # A builder of 1 means the node is supposed to appear
-            # buildable without actually having a builder, so we allow
-            # it to be a side effect as well.
-            if side_effect.has_builder() and side_effect.builder != 1:
+            if side_effect.multiple_side_effect_has_builder():
                 raise SCons.Errors.UserError, "Multiple ways to build the same target were specified for: %s" % str(side_effect)
             side_effect.add_source(targets)
             side_effect.side_effect = 1
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 2f115c9..52a75ce 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -176,10 +176,24 @@ Unlink = SCons.Action.Action(UnlinkFunc, None)
 
 def MkdirFunc(target, source, env):
     t = target[0]
-    t.fs.mkdir(t.path)
+    if not t.fs.exists(t.path):
+        t.fs.mkdir(t.path)
     return 0
 
-Mkdir = SCons.Action.Action(MkdirFunc, None)
+Mkdir = SCons.Action.Action(MkdirFunc, None, presub=None)
+
+MkdirBuilder = None
+
+def get_MkdirBuilder():
+    global MkdirBuilder
+    if MkdirBuilder is None:
+        import SCons.Builder
+        import SCons.Defaults
+        env = SCons.Defaults.DefaultEnvironment()
+        MkdirBuilder = SCons.Builder.Builder(action = Mkdir,
+                                             env = env,
+                                             explain = None)
+    return MkdirBuilder
 
 def CacheRetrieveFunc(target, source, env):
     t = target[0]
@@ -1118,7 +1132,7 @@ class Dir(Base):
         self.entries['.'] = self
         self.entries['..'] = self.dir
         self.cwd = self
-        self.builder = 1
+        self.builder = get_MkdirBuilder()
         self.searched = 0
         self._sconsign = None
         self.build_dirs = []
@@ -1238,7 +1252,13 @@ class Dir(Base):
 
     def build(self, **kw):
         """A null "builder" for directories."""
-        pass
+        global MkdirBuilder
+        if not self.builder is MkdirBuilder:
+            apply(SCons.Node.Node.build, [self,], kw)
+
+    def multiple_side_effect_has_builder(self):
+        global MkdirBuilder
+        return not self.builder is MkdirBuilder and self.has_builder()
 
     def alter_targets(self):
         """Return any corresponding targets in a build directory.
@@ -1262,6 +1282,8 @@ class Dir(Base):
     def current(self, calc=None):
         """If all of our children were up-to-date, then this
         directory was up-to-date, too."""
+        if not self.builder is MkdirBuilder and not self.exists():
+            return 0
         state = 0
         for kid in self.children():
             s = kid.get_state()
@@ -1299,16 +1321,6 @@ class Dir(Base):
             return self.srcdir
         return Base.srcnode(self)
 
-    def get_executor(self, create=1):
-        """Fetch the action executor for this node.  Create one if
-        there isn't already one, and requested to do so."""
-        try:
-            executor = self.executor
-        except AttributeError:
-            executor = DummyExecutor()
-            self.executor = executor
-        return executor
-
     def get_timestamp(self):
         """Return the latest timestamp from among our children"""
         stamp = 0
@@ -1482,8 +1494,12 @@ class File(Base):
         listDirs.reverse()
         for dirnode in listDirs:
             try:
-                Mkdir(dirnode, [], None)
-                # The Mkdir() action may or may not have actually
+                # Don't call dirnode.build(), call the base Node method
+                # directly because we definitely *must* create this
+                # directory.  The dirnode.build() method will suppress
+                # the build if it's the default builder.
+                SCons.Node.Node.build(dirnode)
+                # The build() action may or may not have actually
                 # created the directory, depending on whether the -n
                 # option was used or not.  Delete the _exists and
                 # _rexists attributes so they can be reevaluated.
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index d4137c1..bbc64ef 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -281,8 +281,33 @@ class BuildDirTestCase(unittest.TestCase):
                f8.rfile().path
 
         # Verify the Mkdir and Link actions are called
+        d9 = fs.Dir('build/var2/new_dir')
         f9 = fs.File('build/var2/new_dir/test9.out')
 
+        class MkdirAction(Action):
+            def __init__(self, dir_made):
+                self.dir_made = dir_made
+            def __call__(self, target, source, env, errfunc):
+                self.dir_made.extend(target)
+
+        save_Link = SCons.Node.FS.Link
+        link_made = []
+        def link_func(target, source, env, link_made=link_made):
+            link_made.append(target)
+        SCons.Node.FS.Link = link_func
+
+        try:
+            dir_made = []
+            d9.builder = Builder(fs.Dir, action=MkdirAction(dir_made))
+            f9.exists()
+            expect = os.path.join('build', 'var2', 'new_dir')
+            assert dir_made[0].path == expect, dir_made[0].path
+            expect = os.path.join('build', 'var2', 'new_dir', 'test9.out')
+            assert link_made[0].path == expect, link_made[0].path
+            assert f9.linked
+        finally:
+            SCons.Node.FS.Link = save_Link
+
         # Test for an interesting pathological case...we have a source
         # file in a build path, but not in a source path.  This can
         # happen if you switch from duplicate=1 to duplicate=0, then
@@ -312,29 +337,6 @@ class BuildDirTestCase(unittest.TestCase):
         var2_new_dir = os.path.normpath('build/var2/new_dir')
         assert bdt == [var1_new_dir, var2_new_dir], bdt
 
-        save_Mkdir = SCons.Node.FS.Mkdir
-        dir_made = []
-        def mkdir_func(target, source, env, dir_made=dir_made):
-            dir_made.append(target)
-        SCons.Node.FS.Mkdir = mkdir_func
-
-        save_Link = SCons.Node.FS.Link
-        link_made = []
-        def link_func(target, source, env, link_made=link_made):
-            link_made.append(target)
-        SCons.Node.FS.Link = link_func
-
-        try:
-            f9.exists()
-            expect = os.path.join('build', 'var2', 'new_dir')
-            assert dir_made[0].path == expect, dir_made[0].path
-            expect = os.path.join('build', 'var2', 'new_dir', 'test9.out')
-            assert link_made[0].path == expect, link_made[0].path
-            assert f9.linked
-        finally:
-            SCons.Node.FS.Mkdir = save_Mkdir
-            SCons.Node.FS.Link = save_Link
-
         # Test that an IOError trying to Link a src file
         # into a BuildDir ends up throwing a StopError.
         fIO = fs.File("build/var2/IOError")
@@ -754,7 +756,7 @@ class FSTestCase(unittest.TestCase):
         d1.builder_set(Builder(fs.File))
         d1.env_set(Environment())
         d1.build()
-        assert not built_it
+        assert built_it
 
         built_it = None
         assert not built_it
@@ -1497,22 +1499,24 @@ class prepareTestCase(unittest.TestCase):
             exc_caught = 1
         assert exc_caught, "Should have caught a StopError."
 
-        save_Mkdir = SCons.Node.FS.Mkdir
-        dir_made = []
-        def mkdir_func(target, source, env, dir_made=dir_made):
-            dir_made.append(target)
-        SCons.Node.FS.Mkdir = mkdir_func
+        class MkdirAction(Action):
+            def __init__(self, dir_made):
+                self.dir_made = dir_made
+            def __call__(self, target, source, env, errfunc):
+                self.dir_made.extend(target)
 
-        file = fs.File(os.path.join("new_dir", "xyz"))
-        try:
-            file.set_state(SCons.Node.up_to_date)
-            file.prepare()
-            assert dir_made == [], dir_made
-            file.set_state(0)
-            file.prepare()
-            assert dir_made[0].path == "new_dir", dir_made[0].path
-        finally:
-            SCons.Node.FS.Mkdir = save_Mkdir
+        dir_made = []
+        new_dir = fs.Dir("new_dir")
+        new_dir.builder = Builder(fs.Dir, action=MkdirAction(dir_made))
+        xyz = fs.File(os.path.join("new_dir", "xyz"))
+
+        xyz.set_state(SCons.Node.up_to_date)
+        xyz.prepare()
+        assert dir_made == [], dir_made
+        xyz.set_state(0)
+        xyz.prepare()
+        print "dir_made[0] =", dir_made[0]
+        assert dir_made[0].path == "new_dir", dir_made[0]
 
         dir = fs.Dir("dir")
         dir.prepare()
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index cd7aa18..72ddd74 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -304,6 +304,14 @@ class NodeTestCase(unittest.TestCase):
         n1.builder_set(Builder())
         assert n1.has_builder() == 1
 
+    def test_multiple_side_effect_has_builder(self):
+        """Test the multiple_side_effect_has_builder() method
+        """
+        n1 = SCons.Node.Node()
+        assert n1.multiple_side_effect_has_builder() == 0
+        n1.builder_set(Builder())
+        assert n1.multiple_side_effect_has_builder() == 1
+
     def test_is_derived(self):
         """Test the is_derived() method
         """
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 51e6628..603762e 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -299,6 +299,8 @@ class Node:
             b = self.builder
         return not b is None
 
+    multiple_side_effect_has_builder = has_builder
+
     def is_derived(self):
         """
         Returns true iff this node is derived (i.e. built).
diff --git a/test/Builder-factories.py b/test/Builder-factories.py
new file mode 100644
index 0000000..6007d45
--- /dev/null
+++ b/test/Builder-factories.py
@@ -0,0 +1,72 @@
+#!/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 specify the target_factory and source_factory
+of a Builder.
+"""
+
+import os.path
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+test.subdir('src')
+
+test.write('SConstruct', """
+import os
+import os.path
+def mkdir(env, source, target):
+    t = str(target[0])
+    os.makedirs(t)
+    open(os.path.join(t, 'marker'), 'w').write("MakeDirectory\\n")
+MakeDirectory = Builder(action=mkdir, target_factory=Dir)
+def collect(env, source, target):
+    out = open(str(target[0]), 'w')
+    dir = str(source[0])
+    for f in os.listdir(dir):
+        f = os.path.join(dir, f)
+        out.write(open(f, 'r').read())
+    out.close()
+Collect = Builder(action=collect, source_factory=Dir)
+env = Environment(BUILDERS = {'MakeDirectory':MakeDirectory,
+                              'Collect':Collect})
+env.MakeDirectory('foo', [])
+env.Collect('output', 'src')
+""")
+
+test.write(['src', 'file1'], "src/file1\n")
+test.write(['src', 'file2'], "src/file2\n")
+test.write(['src', 'file3'], "src/file3\n")
+
+test.run(arguments = '.')
+
+test.fail_test(not os.path.isdir(test.workpath('foo')))
+test.must_match(["foo", "marker"], "MakeDirectory\n")
+test.must_match("output", "src/file1\nsrc/file2\nsrc/file3\n")
+
+test.pass_test()