Refactor FunctionAction objects to support -n and -s.

9 files changed, 203 insertions, 112 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index f719883..eebe766 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -19,6 +19,12 @@ RELEASE 0.10 - XXX
 
   - Remove Python bytecode (*.pyc) files from the scons-local packages.
 
+  - Have FunctionActions print a description of what they're doing
+    (a representation of the Python call).
+
+  - Fix the Install() method so that, like other actions, it prints
+    what would have happened when the -n option is used.
+
   From Steve Leblanc:
 
   - Add a Clean() method to support removing user-specified targets
diff --git a/src/RELEASE.txt b/src/RELEASE.txt
index 60594de..0280fed 100644
--- a/src/RELEASE.txt
+++ b/src/RELEASE.txt
@@ -42,6 +42,9 @@ RELEASE 0.10 - XXX
       consistent.  All error messages now begin with "scons: ***"
       and all warning messages now begin with "scons: warning:".
 
+    - SCons now prints a description of Python functions that are
+      invoked to build a target.
+
   Please note the following important changes since release 0.08:
 
     - The SetCommandHandler() function has been superceded
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 8732bd7..d36c860 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -37,9 +37,13 @@ import string
 import sys
 import UserDict
 
-import SCons.Util
 import SCons.Errors
+import SCons.Util
+
+class _Null:
+    pass
 
+_null = _Null
 
 print_actions = 1;
 execute_actions = 1;
@@ -71,7 +75,7 @@ class CommandGenerator:
     def __init__(self, generator):
         self.generator = generator
 
-def _do_create_action(act):
+def _do_create_action(act, strfunction=_null):
     """This is the actual "implementation" for the
     Action factory method, below.  This handles the
     fact that passing lists to Action() itself has
@@ -89,7 +93,7 @@ def _do_create_action(act):
     elif isinstance(act, CommandGenerator):
         return CommandGeneratorAction(act.generator)
     elif callable(act):
-        return FunctionAction(act)
+        return FunctionAction(act, strfunction=strfunction)
     elif SCons.Util.is_String(act):
         var=SCons.Util.get_environment_var(act)
         if var:
@@ -109,17 +113,17 @@ def _do_create_action(act):
     else:
         return None
 
-def Action(act):
+def Action(act, strfunction=_null):
     """A factory for action objects."""
     if SCons.Util.is_List(act):
-        acts = filter(lambda x: not x is None,
-                      map(_do_create_action, act))
+        acts = map(lambda x, s=strfunction: _do_create_action(x, s), act)
+        acts = filter(lambda x: not x is None, acts)
         if len(acts) == 1:
             return acts[0]
         else:
             return ListAction(acts)
     else:
-        return _do_create_action(act)
+        return _do_create_action(act, strfunction=strfunction)
 
 class ActionBase:
     """Base class for actions that create output objects."""
@@ -202,10 +206,10 @@ class CommandAction(ActionBase):
         handle lists of commands, even though that's not how we use it
         externally.
         """
+        import SCons.Util
+
         escape = env.get('ESCAPE', lambda x: x)
 
-        import SCons.Errors
-        
         if env.has_key('SHELL'):
             shell = env['SHELL']
         else:
@@ -271,8 +275,6 @@ class CommandGeneratorAction(ActionBase):
         self.generator = generator
 
     def __generate(self, target, source, env, for_signature):
-        import SCons.Util
-
         # ensure that target is a list, to make it easier to write
         # generator functions:
         if not SCons.Util.is_List(target):
@@ -315,21 +317,45 @@ class LazyCmdGenerator:
 
 class FunctionAction(ActionBase):
     """Class for Python function actions."""
-    def __init__(self, function):
-        self.function = function
+
+    def __init__(self, execfunction, strfunction=_null):
+        self.execfunction = execfunction
+        if strfunction is _null:
+            def strfunction(target, source, execfunction=execfunction):
+                def quote(s):
+                    return '"' + str(s) + '"'
+                try:
+                    name = execfunction.__name__
+                except AttributeError:
+                    try:
+                        name = execfunction.__class__.__name__
+                    except AttributeError:
+                        name = "unknown_python_function"
+                if len(target) == 1:
+                    tstr = quote(target[0])
+                else:
+                    tstr = str(map(lambda x, q=quote: q(x), target))
+                if len(source) == 1:
+                    sstr = quote(source[0])
+                else:
+                    sstr = str(map(lambda x, q=quote: q(x), source))
+                return "%s(%s, %s)" % (name, tstr, sstr)
+        self.strfunction = strfunction
 
     def execute(self, target, source, env):
-        # if print_actions:
-        # XXX:  WHAT SHOULD WE PRINT HERE?
+        r = 0
+        if not SCons.Util.is_List(target):
+            target = [target]
+        if not SCons.Util.is_List(source):
+            source = [source]
+        if print_actions and self.strfunction:
+            s = self.strfunction(target, source)
+            if s:
+                self.show(s)
         if execute_actions:
-            if not SCons.Util.is_List(target):
-                target = [target]
-
-            if not SCons.Util.is_List(source):
-                source = [source]
             rsources = map(rfile, source)
-
-            return self.function(target=target, source=rsources, env=env)
+            r = self.execfunction(target=target, source=rsources, env=env)
+        return r
 
     def get_contents(self, target, source, env):
         """Return the signature contents of this callable action.
@@ -340,11 +366,11 @@ class FunctionAction(ActionBase):
         #XXX DOES NOT ACCOUNT FOR CHANGES IN ENVIRONMENT VARIABLES
         #THE FUNCTION MAY USE
         try:
-            # "self.function" is a function.
-            code = self.function.func_code.co_code
+            # "self.execfunction" is a function.
+            code = self.execfunction.func_code.co_code
         except:
-            # "self.function" is a callable object.
-            code = self.function.__call__.im_func.func_code.co_code
+            # "self.execfunction" is a callable object.
+            code = self.execfunction.__call__.im_func.func_code.co_code
         return str(code)
 
 class ListAction(ActionBase):
@@ -367,9 +393,7 @@ class ListAction(ActionBase):
 
         Simple concatenation of the signatures of the elements.
         """
-
-        ret = ""
-        for a in self.list:
-            ret = ret + a.get_contents(target, source, env)
-        return ret
-
+        return string.join(map(lambda x, t=target, s=source, e=env:
+                                      x.get_contents(t, s, e),
+                               self.list),
+                           "")
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 2f262bf..6c3f779 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -105,9 +105,11 @@ class ActionTestCase(unittest.TestCase):
         """
         def foo():
             pass
+        def bar():
+            pass
         a1 = SCons.Action.Action(foo)
         assert isinstance(a1, SCons.Action.FunctionAction), a1
-        assert a1.function == foo, a1.function
+        assert a1.execfunction == foo, a1.execfunction
 
         a2 = SCons.Action.Action("string")
         assert isinstance(a2, SCons.Action.CommandAction), a2
@@ -156,10 +158,15 @@ class ActionTestCase(unittest.TestCase):
         assert isinstance(a10.list[0], SCons.Action.CommandAction), a10.list[0]
         assert a10.list[0].cmd_list == ["x"], a10.list[0].cmd_list
         assert isinstance(a10.list[1], SCons.Action.FunctionAction), a10.list[1]
-        assert a10.list[1].function == foo, a10.list[1].function
+        assert a10.list[1].execfunction == foo, a10.list[1].execfunction
         assert isinstance(a10.list[2], SCons.Action.CommandAction), a10.list[2]
         assert a10.list[2].cmd_list == ["z"], a10.list[2].cmd_list
 
+        a11 = SCons.Action.Action(foo, strfunction=bar)
+        assert isinstance(a11, SCons.Action.FunctionAction), a11
+        assert a11.execfunction == foo, a11.execfunction
+        assert a11.strfunction == bar, a11.strfunction
+
 class ActionBaseTestCase(unittest.TestCase):
 
     def test_cmp(self):
@@ -487,10 +494,30 @@ class FunctionActionTestCase(unittest.TestCase):
     def test_init(self):
         """Test creation of a function Action
         """
-        def func():
+        def func1():
+            pass
+        def func2():
             pass
-        a = SCons.Action.FunctionAction(func)
-        assert a.function == func
+        def func3():
+            pass
+        def func4():
+            pass
+
+        a = SCons.Action.FunctionAction(func1)
+        assert a.execfunction == func1, a.execfunction
+        assert isinstance(a.strfunction, types.FunctionType)
+
+        a = SCons.Action.FunctionAction(func2, strfunction=func3)
+        assert a.execfunction == func2, a.execfunction
+        assert a.strfunction == func3, a.strfunction
+
+        a = SCons.Action.FunctionAction(func3, func4)
+        assert a.execfunction == func3, a.execfunction
+        assert a.strfunction == func4, a.strfunction
+
+        a = SCons.Action.FunctionAction(func4, None)
+        assert a.execfunction == func4, a.execfunction
+        assert a.strfunction is None, a.strfunction
 
     def test_execute(self):
         """Test executing a function Action
@@ -555,6 +582,18 @@ class FunctionActionTestCase(unittest.TestCase):
         c = test.read(outfile, 'r')
         assert c == "class1b\n", c
 
+        def build_it(target, source, env, self=self):
+            self.build_it = 1
+            return 0
+        def string_it(target, source, self=self):
+            self.string_it = 1
+            return None
+        act = SCons.Action.FunctionAction(build_it, string_it)
+        r = act.execute([], [], Environment())
+        assert r == 0, r
+        assert self.build_it
+        assert self.string_it
+
     def test_get_contents(self):
         """Test fetching the contents of a function Action
         """
@@ -677,7 +716,7 @@ if __name__ == "__main__":
                  CommandGeneratorActionTestCase,
                  FunctionActionTestCase,
                  ListActionTestCase,
-                 LazyActionTestCase]
+                 LazyActionTestCase ]
     for tclass in tclasses:
         names = unittest.getTestCaseNames(tclass, 'test_')
         suite.addTests(map(tclass, names))
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index a2369b9..5906314 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -36,6 +36,8 @@ import types
 import unittest
 
 import TestCmd
+
+import SCons.Action
 import SCons.Builder
 import SCons.Errors
 import SCons.Node.FS
@@ -188,7 +190,11 @@ class BuilderTestCase(unittest.TestCase):
         def func():
             pass
         builder = SCons.Builder.Builder(name="builder", action=func)
-        assert builder.action.function == func
+        assert isinstance(builder.action, SCons.Action.FunctionAction)
+        # Preserve the following so that the baseline test will fail.
+        # Remove it in favor of the previous test at some convenient
+        # point in the future.
+        assert builder.action.execfunction == func
 
     def test_generator(self):
         """Test Builder creation given a generator function."""
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index ec8f1eb..480169e 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -38,40 +38,25 @@ import shutil
 import string
 import sys
 import types
+from UserDict import UserDict
 
+import SCons.Action
 import SCons.Builder
 import SCons.Defaults
-from SCons.Errors import UserError
+import SCons.Errors
 import SCons.Node
 import SCons.Node.FS
-import SCons.Util
-import SCons.Warnings
-from UserDict import UserDict
 import SCons.Platform
 import SCons.Tool
+import SCons.Util
+import SCons.Warnings
+
+def installString(target, source):
+    return 'Install file: "%s" as "%s"' % (source[0], target[0])
+
+installAction = SCons.Action.Action(SCons.Node.FS.LinkFunc, installString)
 
-def installFunc(target, source, env):
-    try:
-        map(lambda t: os.unlink(str(t)), target)
-    except OSError:
-        pass
-
-    try:
-        SCons.Node.FS.file_link(str(source[0]), str(target[0]))
-        print 'Install file: "%s" as "%s"' % \
-              (source[0], target[0])
-        return 0
-    except IOError, e:
-        sys.stderr.write('Unable to install "%s" as "%s"\n%s\n' % \
-                         (source[0], target[0], str(e)))
-        return -1
-    except OSError, e:
-        sys.stderr.write('Unable to install "%s" as "%s"\n%s\n' % \
-                         (source[0], target[0], str(e)))
-        return -1
-
-InstallBuilder = SCons.Builder.Builder(name='Install',
-                                       action=installFunc)
+InstallBuilder = SCons.Builder.Builder(name='Install', action=installAction)
 
 def our_deepcopy(x):
    """deepcopy lists and dictionaries, and just copy the reference
@@ -232,7 +217,7 @@ class Environment:
                 for name, builder in bd.items():
                     setattr(self, name, BuilderWrapper(self, builder))
             else:
-                raise UserError, "The use of the BUILDERS Environment variable as a list or Builder instance is deprecated.  BUILDERS should be a dictionary of name->Builder instead."
+                raise SCons.Errors.UserError, "The use of the BUILDERS Environment variable as a list or Builder instance is deprecated.  BUILDERS should be a dictionary of name->Builder instead."
         for s in self._dict['SCANNERS']:
             setattr(self, s.name, s)
         
@@ -378,7 +363,7 @@ class Environment:
 	    # buildable without actually having a builder, so we allow
 	    # it to be a side effect as well.
             if side_effect.builder is not None and side_effect.builder != 1:
-                raise UserError, "Multiple ways to build the same target were specified for: %s" % str(side_effect)
+                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
             self.Precious(side_effect)
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 5c01c9d..626a22a 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -36,19 +36,35 @@ canonical default.
 
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
-import string
 import os
 import os.path
+import string
+import sys
 import types
-import SCons.Node
 from UserDict import UserDict
-import sys
+
+import SCons.Action
 import SCons.Errors
+import SCons.Node
 import SCons.Warnings
 
-execute_actions = 1
+#
+# SCons.Action objects for interacting with the outside world.
+#
+# The Node.FS methods in this module should use these actions to
+# create and/or remove files and directories; they should *not* use
+# os.{link,symlink,unlink,mkdir}(), etc., directly.
+#
+# Using these SCons.Action objects ensures that descriptions of these
+# external activities are properly displayed, that the displays are
+# suppressed when the -s (silent) option is used, and (most importantly)
+# the actions are disabled when the the -n option is used, in which case
+# there should be *no* changes to the external file system(s)...
+#
 
-def file_link(src, dest):
+def LinkFunc(target, source, env):
+    src = source[0].path
+    dest = target[0].path
     dir, file = os.path.split(dest)
     if dir and not os.path.isdir(dir):
         os.makedirs(dir)
@@ -56,17 +72,37 @@ def file_link(src, dest):
     # fails, try a symlink.  If that fails then just copy it.
     try :
         os.link(src, dest)
-    except (AttributeError, OSError) :
+    except (AttributeError, OSError):
         try :
             os.symlink(src, dest)
-        except (AttributeError, OSError) :
+        except (AttributeError, OSError):
             import shutil
             import stat
             shutil.copy2(src, dest)
             st=os.stat(src)
             os.chmod(dest, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
+    return 0
+
+LinkAction = SCons.Action.Action(LinkFunc, None)
+
+def LocalString(target, source):
+    return 'Local copy of %s from %s' % (target[0], source[0])
+
+LocalCopy = SCons.Action.Action(LinkFunc, LocalString)
+
+def UnlinkFunc(target, source, env):
+    os.unlink(target[0].path)
+    return 0
 
+UnlinkAction = SCons.Action.Action(UnlinkFunc, None)
 
+def MkdirFunc(target, source, env):
+    os.mkdir(target[0].path)
+    return 0
+
+MkdirAction = SCons.Action.Action(MkdirFunc, None)
+
+#
 class ParentOfRoot:
     """
     An instance of this class is used as the parent of the root of a
@@ -842,13 +878,11 @@ class File(Entry):
             if isinstance(p, ParentOfRoot):
                 raise SCons.Errors.StopError, parent.path
             parent = p
-        if not execute_actions:
-            return
         listDirs.reverse()
         for dirnode in listDirs:
+            dirnode._exists = 1
             try:
-                os.mkdir(dirnode.abspath)
-                dirnode._exists = 1
+                MkdirAction.execute(dirnode, None, None)
             except OSError:
                 pass
 
@@ -871,8 +905,7 @@ class File(Entry):
 
         if self.exists():
             if self.builder and not self.precious:
-                if execute_actions:
-                    os.unlink(self.path)
+                UnlinkAction.execute(self, None, None)
                 if hasattr(self, '_exists'):
                     delattr(self, '_exists')
         else:
@@ -894,12 +927,12 @@ class File(Entry):
         if self.duplicate and not self.builder and not self.created:
             src=self.srcnode().rfile()
             if src.exists() and src.abspath != self.abspath:
+                self._createDir()
                 try:
-                    os.unlink(self.abspath)
+                    UnlinkAction.execute(self, None, None)
                 except OSError:
                     pass
-                self._createDir()
-                file_link(src.abspath, self.abspath)
+                LinkAction.execute(self, src, None)
                 self.created = 1
 
                 # Set our exists cache accordingly
@@ -919,9 +952,7 @@ class File(Entry):
                     # ...and it's even up-to-date...
                     if self._local:
                         # ...and they'd like a local copy.
-                        print "Local copy of %s from %s" % (self.path, r.path)
-                        if execute_actions:
-                            file_link(r.path, self.path)
+                        LocalCopy.execute(self, r, None)
                         self.set_bsig(bsig)
                         self.store_bsig()
                     return 1
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index 43fa41f..98857bf 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -90,7 +90,6 @@ class BuildDirTestCase(unittest.TestCase):
     def runTest(self):
         """Test build dir functionality"""
         test=TestCmd(workdir='')
-        os.chdir(test.workdir)
 
         fs = SCons.Node.FS.FS()
         f1 = fs.File('build/test1')
@@ -157,6 +156,8 @@ class BuildDirTestCase(unittest.TestCase):
         test.write([ 'rep1', 'build', 'var1', 'test2.out' ], 'test2.out_rep')
         test.write([ 'rep1', 'build', 'var2', 'test2.out' ], 'test2.out_rep')
 
+        os.chdir(test.workpath('work'))
+
         fs = SCons.Node.FS.FS(test.workpath('work'))
         fs.BuildDir('build/var1', 'src', duplicate=0)
         fs.BuildDir('build/var2', 'src')
@@ -254,12 +255,13 @@ class BuildDirTestCase(unittest.TestCase):
         assert f8.rfile().path == os.path.normpath(test.workpath('rep1/build/var2/test2.out')),\
                f8.rfile().path
         
-        # Test to see if file_link() works...
+        # Test to see if LinkAction() works...
         test.subdir('src','build')
-        test.write('src/foo', 'foo\n')
+        test.write('src/foo', 'src/foo\n')
         os.chmod(test.workpath('src/foo'), stat.S_IRUSR)
-        SCons.Node.FS.file_link(test.workpath('src/foo'),
-                                test.workpath('build/foo'))
+        SCons.Node.FS.LinkAction.execute(fs.File(test.workpath('build/foo')),
+                                         fs.File(test.workpath('src/foo')),
+                                         None)
         os.chmod(test.workpath('src/foo'), stat.S_IRUSR | stat.S_IWRITE)
         st=os.stat(test.workpath('build/foo'))
         assert (stat.S_IMODE(st[stat.ST_MODE]) & stat.S_IWRITE), \
@@ -352,14 +354,11 @@ class BuildDirTestCase(unittest.TestCase):
         os.symlink = simulator.symlink_fail
         shutil.copy2 = simulator.copy
 
-        # XXX this is just to pass the baseline test, it won't be needed once
-        # this change is integrated
-        SCons.Node.FS._link = simulator.link_fail
-
-        test.write('src/foo', 'foo\n')
+        test.write('src/foo', 'src/foo\n')
         os.chmod(test.workpath('src/foo'), stat.S_IRUSR)
-        SCons.Node.FS.file_link(test.workpath('src/foo'),
-                                test.workpath('build/foo'))
+        SCons.Node.FS.LinkAction.execute(fs.File(test.workpath('build/foo')),
+                                         fs.File(test.workpath('src/foo')),
+                                         None)
         test.unlink( "src/foo" )
         test.unlink( "build/foo" )
 
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index b4afb3c..093b82c 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -55,17 +55,16 @@ import copy
 #                         'lib',
 #                         'scons-%d' % SCons.__version__)] + sys.path[1:]
 
+import SCons.Errors
+import SCons.Job
 import SCons.Node
 import SCons.Node.FS
-import SCons.Job
-from SCons.Errors import *
-import SCons.Sig
-from SCons.Taskmaster import Taskmaster
-import SCons.Builder
-import SCons.Script.SConscript
-import SCons.Warnings
 from SCons.Optik import OptionParser, SUPPRESS_HELP, OptionValueError
+import SCons.Script.SConscript
+import SCons.Sig
+import SCons.Taskmaster
 from SCons.Util import display
+import SCons.Warnings
 
 
 #
@@ -124,15 +123,15 @@ class BuildTask(SCons.Taskmaster.Task):
 
     def failed(self):
         e = sys.exc_value
-        if sys.exc_type == BuildError:
+        if sys.exc_type == SCons.Errors.BuildError:
             sys.stderr.write("scons: *** [%s] %s\n" % (e.node, e.errstr))
             if e.errstr == 'Exception':
                 traceback.print_exception(e.args[0], e.args[1], e.args[2])
-        elif sys.exc_type == UserError:
+        elif sys.exc_type == SCons.Errors.UserError:
             # We aren't being called out of a user frame, so
             # don't try to walk the stack, just print the error.
             sys.stderr.write("\nscons: *** %s\n" % e)
-        elif sys.exc_type == StopError:
+        elif sys.exc_type == SCons.Errors.StopError:
             s = str(e)
             if not keep_going_on_error:
                 s = s + '  Stop.'
@@ -661,7 +660,6 @@ def _main():
         _setup_warn(options.warn)
     if options.noexec:
         SCons.Action.execute_actions = None
-        SCons.Node.FS.execute_actions = None
         CleanTask.execute = CleanTask.show
     if options.no_progress or options.silent:
         display.set_mode(0)
@@ -696,7 +694,7 @@ def _main():
             display("scons: Entering directory %s" % script_dir)
             os.chdir(script_dir)
         else:
-            raise UserError, "No SConstruct file found."
+            raise SCons.Errors.UserError, "No SConstruct file found."
 
     SCons.Node.FS.default_fs.set_toplevel_dir(os.getcwd())
 
@@ -718,7 +716,7 @@ def _main():
         SCons.Script.SConscript.print_help = 1
 
     if not scripts:
-        raise UserError, "No SConstruct file found."
+        raise SCons.Errors.UserError, "No SConstruct file found."
 
     class Unbuffered:
         def __init__(self, file):
@@ -863,7 +861,7 @@ def main():
         sys.exit(2)
     except SyntaxError, e:
         _scons_syntax_error(e)
-    except UserError, e:
+    except SCons.Errors.UserError, e:
         _scons_user_error(e)
     except:
         _scons_other_errors()