Make FunctionAction arguments be Nodes, not strings. (Charles Crain)

15 files changed, 80 insertions, 48 deletions

diff --git a/README b/README
index 82ae6f6..014b22b 100644
--- a/README
+++ b/README
@@ -55,7 +55,7 @@ system, populate the build/scons/ directory by running:
 
         $ scons build/scons
 
-If you don't have SCons version 0.04 or later already installed on your
+If you don't have SCons version 0.06 or later already installed on your
 system, you can use SCons itself to populate the build/scons/ directory
 with a little more work:
 
@@ -124,13 +124,13 @@ the "TESTING PACKAGES" section below.
 BUILDING PACKAGES
 =================
 
-We now use SCons (version 0.04 or later) to build its own packages.
+We now use SCons (version 0.06 or later) to build its own packages.
 If you already have an appropriate version of SCons installed on your
 system, you can build everything by simply running it:
 
         $ scons
 
-If you don't have SCons version 0.04 or later already installed on your
+If you don't have SCons version 0.06 or later already installed on your
 system, you can build this version of SCons with itself with a little
 more work:
 
diff --git a/SConstruct b/SConstruct
index 3503874..5303c70 100644
--- a/SConstruct
+++ b/SConstruct
@@ -201,26 +201,28 @@ def SCons_revision(target, source, env):
     This is used to copy files into a tree that gets packaged up
     into the source file package.
     """
+    t = str(target[0])
+    s = str(source[0])
     # Note:  We don't use $VERSION from the environment so that
     # this routine will change when the version number changes
     # and things will get rebuilt properly.
     global version
-    print "SCons_revision() < %s > %s" % (source[0], target)
-    inf = open(source[0], 'rb')
-    outf = open(target, 'wb')
+    print "SCons_revision() < %s > %s" % (s, t)
+    inf = open(s, 'rb')
+    outf = open(t, 'wb')
     for line in inf.readlines():
         # Note:  We construct the __*__ substitution strings here
         # so that they don't get replaced when this file gets
         # copied into the tree for packaging.
         line = string.replace(line, '_' + '_DATE__', env['DATE'])
         line = string.replace(line, '_' + '_DEVELOPER__', env['DEVELOPER'])
-        line = string.replace(line, '_' + '_FILE__', source[0])
+        line = string.replace(line, '_' + '_FILE__', s)
         line = string.replace(line, '_' + '_REVISION__', env['REVISION'])
         line = string.replace(line, '_' + '_VERSION__', version)
         outf.write(line)
     inf.close()
     outf.close()
-    os.chmod(target, os.stat(source[0])[0])
+    os.chmod(t, os.stat(s)[0])
 
 revbuilder = Builder(name = 'SCons_revision', action = SCons_revision)
 
@@ -488,7 +490,7 @@ for p in [ scons ]:
     def copy(target, source, **kw):
         global src_files
 	src_files.sort()
-        f = open(target, 'wb')
+        f = open(str(target[0]), 'wb')
         for file in src_files:
             f.write(file + "\n")
         f.close()
diff --git a/doc/man/scons.1 b/doc/man/scons.1
index 6e4b72b..5d8a0a5 100644
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -1285,9 +1285,9 @@ If the argument is a Python function,
 a function Action is returned.
 The Python function takes three keyword arguments,
 .B target
-(the name of the target file),
+(a Node object representing the target file),
 .B source
-(the name of the source file)
+(a Node object representing the source file)
 and
 .BR env
 (the construction environment
@@ -1296,8 +1296,15 @@ The
 .B target
 and
 .B source
-arguments may be lists of strings if there is
+arguments may be lists of Node objects if there is
 more than one target file or source file.
+The actual target and source file name(s) may
+be retrieved from their Node objects
+via the built-in Python str() function:
+.ES
+target_file_name = str(target)
+source_file_names = map(lambda x: str(x), source)
+.EE
 .IP
 The function should return
 .B 0
diff --git a/etc/SConscript b/etc/SConscript
index a55fe67..96a464d 100644
--- a/etc/SConscript
+++ b/etc/SConscript
@@ -36,8 +36,10 @@ files = [
 ]
 
 def copy(target, source, env):
-    print "copy() < %s > %s" % (source[0], target)
-    open(target, 'wb').write(open(source[0], 'rb').read())
+    t = str(target[0])
+    s = str(source[0])
+    print "copy() < %s > %s" % (s, t)
+    open(t, 'wb').write(open(s, 'rb').read())
 
 for file in files:
     # Guarantee that real copies of these files always exist in
diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 46177bd..70ba009 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -10,6 +10,12 @@
 
 RELEASE 0.06 - 
 
+  From Charles Crain:
+
+  - Fix command generators to expand construction variables.
+
+  - Make FunctionAction arguments be Nodes, not strings.
+
   From Stephen Kennedy:
 
   - Performance:  Use a dictionary, not a list, for a Node's parents.
diff --git a/src/RELEASE.txt b/src/RELEASE.txt
index 60aca86..733a2ef 100644
--- a/src/RELEASE.txt
+++ b/src/RELEASE.txt
@@ -20,11 +20,25 @@ more effectively, please sign up for the scons-users mailing list at:
 
 
 
-RELEASE 0.05 - Thu, 21 Feb 2002 16:50:03 -0600
+RELEASE 0.06 - 
 
-  This is the fifth alpha release of SCons.  Please consult the
+  This is the sixth alpha release of SCons.  Please consult the
   CHANGES.txt file for a list of specific changes since last release.
 
+  Please note the following important changes since the previous
+  release:
+
+    - Python functions as Builder actions now take Node objects, not
+      strings, as arguments.  The string representation of a Node
+      object is the file name, so you should change your function
+      actions to use the str() built-in function to fetch the file
+      name:
+
+        def build_it(target = None, source = None, env = None):
+            print "Building %s from %s" % (str(target), str(source))
+	    open(str(target), 'w').write(open(str(source), 'r').read())
+            return 0
+
   Owing to an extensive test suite, the SCons team believes that this
   release is of sufficient quality that you can use it for real work,
   despite the "alpha" label.
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 26595d6..503fe15 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -383,13 +383,12 @@ class FunctionAction(ActionBase):
         # if print_actions:
         # XXX:  WHAT SHOULD WE PRINT HERE?
         if execute_actions:
-            if kw.has_key('target'):
-                if SCons.Util.is_List(kw['target']):
-                    kw['target'] = map(str, kw['target'])
-                else:
-                    kw['target'] = str(kw['target'])
-            if kw.has_key('source'):
-                kw['source'] = map(str, kw['source'])
+            if kw.has_key('target') and not \
+               SCons.Util.is_List(kw['target']):
+                kw['target'] = [ kw['target'] ]
+            if kw.has_key('source') and not \
+               SCons.Util.is_List(kw['source']):
+                kw['source'] = [ kw['source'] ]
             return apply(self.function, (), kw)
 
     def get_contents(self, **kw):
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 592fa82..8b417a8 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -203,12 +203,16 @@ class FunctionActionTestCase(unittest.TestCase):
         """Test executing a function Action
         """
         self.inc = 0
-        def f(s):
+        def f(s, target, source):
             s.inc = s.inc + 1
+            s.target = target
+            s.source=source
             return 0
         a = SCons.Action.FunctionAction(f)
-        a.execute(s = self)
+        a.execute(s = self, target=1, source=2)
         assert self.inc == 1, self.inc
+        assert self.source == [2], self.source
+        assert self.target == [1], self.target
 
     def test_get_contents(self):
         """Test fetching the contents of a function Action
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index fc530ab..ab63317 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -513,12 +513,10 @@ class BuilderTestCase(unittest.TestCase):
         def function2(tlist = [outfile, outfile2], **kw):
             global count
             count = count + 1
-            if not type(kw['target']) is type([]):
-                kw['target'] = [ kw['target'] ]
             for t in kw['target']:
-                open(t, 'w').write("function2\n")
+                open(str(t), 'w').write("function2\n")
             for t in tlist:
-                if not t in kw['target']:
+                if not t in map(str, kw['target']):
                     open(t, 'w').write("function2\n")
             return 1
 
@@ -543,12 +541,10 @@ class BuilderTestCase(unittest.TestCase):
         def function3(tlist = [sub1_out, sub2_out], **kw):
             global count
             count = count + 1
-            if not type(kw['target']) is type([]):
-                kw['target'] = [ kw['target'] ]
             for t in kw['target']:
-                open(t, 'w').write("function3\n")
+                open(str(t), 'w').write("function3\n")
             for t in tlist:
-                if not t in kw['target']:
+                if not t in map(str, kw['target']):
                     open(t, 'w').write("function3\n")
             return 1
 
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 49249f3..d90e029 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -45,22 +45,22 @@ import shutil
 
 def installFunc(env, target, source):
     try:
-        os.unlink(target)
+        map(lambda t: os.unlink(str(t)), target)
     except OSError:
         pass
     
     try:
-        SCons.Node.FS.file_link(source[0], target)
+        SCons.Node.FS.file_link(str(source[0]), str(target[0]))
         print 'Install file: "%s" as "%s"' % \
-              (source[0], target)
+              (source[0], target[0])
         return 0
     except IOError, e:
         sys.stderr.write('Unable to install "%s" as "%s"\n%s\n' % \
-                         (source[0], target, str(e)))
+                         (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, str(e)))
+                         (source[0], target[0], str(e)))
         return -1
 
 InstallBuilder = SCons.Builder.Builder(name='Install',
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index 40945a1..4917c83 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -295,8 +295,8 @@ class EnvironmentTestCase(unittest.TestCase):
         assert 'foo2.in' in map(lambda x: x.path, t.sources)
 
         def testFunc(env, target, source):
-            assert target == 'foo.out'
-            assert 'foo1.in' in source and 'foo2.in' in source, source
+            assert str(target[0]) == 'foo.out'
+            assert 'foo1.in' in map(str, source) and 'foo2.in' in map(str, source), map(str, source)
             return 0
         t = env.Command(target='foo.out', source=['foo1.in','foo2.in'],
                         action=testFunc)
diff --git a/test/BuildDir.py b/test/BuildDir.py
index 63b357e..a71d4dd 100644
--- a/test/BuildDir.py
+++ b/test/BuildDir.py
@@ -68,8 +68,8 @@ import os.path
 def buildIt(target, source, env):
     if not os.path.exists('build'):
         os.mkdir('build')
-    f1=open(source[0], 'r')
-    f2=open(target, 'w')
+    f1=open(str(source[0]), 'r')
+    f2=open(str(target[0]), 'w')
     f2.write(f1.read())
     f2.close()
     f1.close()
diff --git a/test/Command.py b/test/Command.py
index ddf78ce..1f4a490 100644
--- a/test/Command.py
+++ b/test/Command.py
@@ -41,8 +41,8 @@ file.close()
 
 test.write('SConstruct', """
 def buildIt(env, target, source):
-    contents = open(source[0], 'rb').read()
-    file = open(target, 'wb')
+    contents = open(str(source[0]), 'rb').read()
+    file = open(str(target[0]), 'wb')
     file.write(contents)
     file.close()
     return 0
diff --git a/test/actions.py b/test/actions.py
index c6198dd..65fa01a 100644
--- a/test/actions.py
+++ b/test/actions.py
@@ -70,7 +70,8 @@ test.write('SConstruct', """
 import os
 import string
 def func(env, target, source):
-    cmd = r'%s build.py %%s 3 %%s' %% (target, string.join(source, ' '))
+    cmd = r'%s build.py %%s 3 %%s' %% (string.join(map(str, target)),
+                                       string.join(map(str, source)))
     print cmd
     return os.system(cmd)
 B = Builder(name = 'B', action = func)
@@ -96,7 +97,8 @@ class bld:
 	print cmd
         return os.system(cmd)
     def get_contents(self, env, target, source):
-        return self.cmd %% (target, string.join(source, ' '))
+        return self.cmd %% (string.join(map(str, target)),
+                            string.join(map(str, source)))
 B = Builder(name = 'B', action = bld())
 env = Environment(BUILDERS = [B])
 env.B(target = 'foo.out', source = 'foo.in')
diff --git a/test/sconsign.py b/test/sconsign.py
index 396c399..a7c6971 100644
--- a/test/sconsign.py
+++ b/test/sconsign.py
@@ -39,8 +39,8 @@ def build1(target, source, env):
 def build2(target, source, env):
     import os
     import os.path
-    open(str(target), 'wb').write(open(str(source[0]), 'rb').read())
-    dir, file = os.path.split(target)
+    open(str(target[0]), 'wb').write(open(str(source[0]), 'rb').read())
+    dir, file = os.path.split(str(target[0]))
     os.chmod(dir, 0555)
     return None