Fix --debug=explain when the action is a Python function.

7 files changed, 105 insertions, 12 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index d7b6693..ee2c546 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -53,6 +53,10 @@ RELEASE 0.97 - XXX
     (already supported by Conftest.py) to specify text at the top of
     the compiled test file.
 
+  - Fix the --debug=explain output when a Python function action changed
+    so it prints a meaningful string, not the binary representation of
+    the function contents.
+
   From Elliot Murphy:
 
   - Enhance the tests to guarantee persistence of ListOption
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index e110d60..c131693 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -37,6 +37,11 @@ other modules:
         to compare the actions used to build a target last time and
         this time.
 
+    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:
 
@@ -46,11 +51,6 @@ this module:
         the pre-substitution command whenever the --debug=presub option
         is used.
 
-    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).
-
     execute()
         The internal method that really, truly, actually handles the
         execution of a command or Python function.  This is used so
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index b3e6f88..bfb1a65 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -137,6 +137,26 @@ class Executor:
                                            self.get_build_env())
             return self.string
 
+    def strfunction(self):
+        try:
+            return self.string
+        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 get_raw_contents(self):
         """Fetch the raw signature contents.  This, along with
         get_contents(), is the real reason this class exists, so we can
diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py
index c4a5552..c391543 100644
--- a/src/engine/SCons/ExecutorTests.py
+++ b/src/engine/SCons/ExecutorTests.py
@@ -48,6 +48,8 @@ class MyAction:
         self.actions = actions
     def get_actions(self):
         return self.actions
+    def strfunction(self, target, source, env):
+        return string.join(['STRFUNCTION'] + self.actions + target + source)
     def genstring(self, target, source, env):
         return string.join(['GENSTRING'] + self.actions + target + source)
     def get_raw_contents(self, target, source, env):
@@ -195,6 +197,14 @@ 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_get_raw_contents(self):
         """Test fetching the raw signatures contents"""
         env = MyEnvironment(RC='raw contents')
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 0638375..47139fc 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -545,7 +545,7 @@ class Node:
 
         if self.has_builder():
             executor = self.get_executor()
-            binfo.bact = executor.get_contents()
+            binfo.bact = executor.strfunction()
             binfo.bactsig = calc.module.signature(executor)
             sigs.append(binfo.bactsig)
 
@@ -899,10 +899,13 @@ class Node:
 
         if len(lines) == 0:
             newact, newactsig = self.binfo.bact, self.binfo.bactsig
-            if old.bact != newact:
-                lines.append("the build action changed:\n" +
-                             "%sold: %s\n" % (' '*15, old.bact) +
-                             "%snew: %s\n" % (' '*15, newact))
+            if old.bactsig != newactsig:
+                if old.bact == newact:
+                    lines.append("the contents of the build action changed\n")
+                else:
+                    lines.append("the build action changed:\n" +
+                                 "%sold: %s\n" % (' '*15, old.bact) +
+                                 "%snew: %s\n" % (' '*15, newact))
 
         if len(lines) == 0:
             return "rebuilding `%s' for unknown reasons" % self
diff --git a/test/explain.py b/test/explain.py
index ae4a33a..be966c0 100644
--- a/test/explain.py
+++ b/test/explain.py
@@ -38,7 +38,8 @@ python = TestSCons.python
 test = TestSCons.TestSCons()
 
 test.subdir('work1', ['work1', 'src'], ['work1', 'src', 'subdir'],
-            'work4', ['work4', 'src'], ['work4', 'src', 'subdir'])
+            'work4', ['work4', 'src'], ['work4', 'src', 'subdir'],
+            'work5')
 
 subdir_file6 = os.path.join('subdir', 'file6')
 subdir_file6_in = os.path.join('subdir', 'file6.in')
@@ -442,4 +443,59 @@ ccc 1
 file5.k 1 line 4
 """)
 
+
+
+test.write(['work5', 'SConstruct'], """\
+import shutil
+
+env = Environment()
+mode = int(ARGUMENTS.get('mode'))
+if mode:
+    def DifferentCopy(target, source, env):
+        tgt = str(target[0])
+        src = str(source[0])
+        shutil.copy(src, tgt)
+    MyCopy = Builder(action = DifferentCopy)
+
+    def ChangingCopy(target, source, env):
+        tgt = str(target[0])
+        src = str(source[0])
+        shutil.copy(src, tgt)
+    ChangingCopy = Builder(action = ChangingCopy)
+else:
+    MyCopy = Builder(action = Copy('$TARGET', '$SOURCE'))
+    def ChangingCopy(target, source, env):
+        tgt = str(target[0].abspath)
+        src = str(source[0].abspath)
+        shutil.copy(src, tgt)
+    ChangingCopy = Builder(action = ChangingCopy)
+
+env['BUILDERS']['MyCopy'] = MyCopy
+env['BUILDERS']['ChangingCopy'] = ChangingCopy
+
+env.MyCopy('f1.out', 'f1.in')
+env.ChangingCopy('f2.out', 'f2.in')
+""")
+
+test.write(['work5', 'f1.in'], "work5/f1.in\n")
+test.write(['work5', 'f2.in'], "work5/f2.in\n")
+
+test.run(chdir = 'work5', arguments = "mode=0 .")
+
+test.must_match(['work5', 'f1.out'], "work5/f1.in\n")
+test.must_match(['work5', 'f2.out'], "work5/f2.in\n")
+
+test.run(chdir = 'work5',
+         arguments = "--debug=explain mode=1 .",
+         stdout = test.wrap_stdout("""\
+scons: rebuilding `f1.out' because the build action changed:
+               old: Copy("f1.out", "f1.in")
+               new: DifferentCopy(["f1.out"], ["f1.in"])
+DifferentCopy(["f1.out"], ["f1.in"])
+scons: rebuilding `f2.out' because the contents of the build action changed
+ChangingCopy(["f2.out"], ["f2.in"])
+"""))
+
+
+
 test.pass_test()
diff --git a/test/symlink.py b/test/symlink.py
index 62ce75b..889c7a0 100644
--- a/test/symlink.py
+++ b/test/symlink.py
@@ -59,7 +59,7 @@ expect = "scons: *** [%s] Error 1\n" % foo_obj
 test.fail_test(string.find(test.stderr(), expect) == -1)
 
 test.write('SConstruct', """
-Command('file.out', 'file.in', Copy())
+Command('file.out', 'file.in', Copy('$TARGET', '$SOURCE'))
 """)
 
 test.symlink('nonexistent', 'file.in')