Fix SideEffect() and BuildDir(). (Anthony Roach)

5 files changed, 74 insertions, 21 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 509ed4c..3ae491f 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -115,6 +115,9 @@ RELEASE 0.14 - XXX
   - Fix a number of tests that searched for a Fortran compiler using the
     external PATH instead of what SCons would use.
 
+  - Fix the interaction of SideEffect() and BuildDir() so that (for
+    example) PDB files get put correctly in a BuildDir().
+
   From David Snopek:
 
   - Contribute the "Autoscons" code for Autoconf-like checking for
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index d0cb379..1af6032 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -350,7 +350,7 @@ class Entry(SCons.Node.Node):
 
     def __str__(self):
         """A FS node's string representation is its path name."""
-        if self.duplicate or self.has_builder():
+        if self.duplicate or self.is_derived():
             return self.get_path()
         return self.srcnode().get_path()
 
@@ -766,7 +766,7 @@ class FS:
 			# This is usually the case with BuildDir().
 			# We only want to find pre-existing files.
                         if rnode.exists() and \
-                           (isinstance(rnode, Dir) or not rnode.has_builder()):
+                           (isinstance(rnode, Dir) or not rnode.is_derived()):
                             return rnode
                     except TypeError:
                         pass # Wrong type of node.
@@ -814,7 +814,7 @@ class FS:
                             # with BuildDir().  We only want to find
                             # pre-existing files.
                             if (not must_exist or rnode.exists()) and \
-                               (not rnode.has_builder() or isinstance(rnode, Dir)):
+                               (not rnode.is_derived() or isinstance(rnode, Dir)):
                                 ret.append(rnode)
                         except TypeError:
                             pass # Wrong type of node.
@@ -1122,7 +1122,7 @@ class File(Entry):
         returns - the signature
         """
 
-        if self.has_builder():
+        if self.is_derived():
             if SCons.Sig.build_signature:
                 return calc.bsig(self.rfile(), self)
             else:
@@ -1226,7 +1226,7 @@ class File(Entry):
         so only do thread safe stuff here. Do thread unsafe stuff in
         built().
         """
-        b = self.has_builder()
+        b = self.is_derived()
         if not b and not self.has_src_builder():
             return
         if b and self.fs.CachePath:
@@ -1298,22 +1298,16 @@ class File(Entry):
             self.sbuilder = scb
         return not scb is None
 
-    def is_derived(self):
-        """Return whether this file is a derived file or not.
-
-        This overrides the base class method to account for the fact
-        that a file may be derived transparently from a source code
-        builder.
-        """
-        return self.has_builder() or self.side_effect or self.has_src_builder()
-
     def alter_targets(self):
         """Return any corresponding targets in a build directory.
         """
-        if self.has_builder():
+        if self.is_derived():
             return [], None
         return self.fs.build_dir_target_climb(self.dir, [self.name])
 
+    def is_pseudo_derived(self):
+        return self.has_src_builder()
+    
     def prepare(self):
         """Prepare for this file to be created."""
 
@@ -1321,7 +1315,7 @@ class File(Entry):
 
         if self.get_state() != SCons.Node.up_to_date:
             if self.exists():
-                if self.has_builder() and not self.precious:
+                if self.is_derived() and not self.precious:
                     try:
                         Unlink(self, None, None)
                     except OSError, e:
@@ -1345,7 +1339,7 @@ class File(Entry):
 
     def exists(self):
         # Duplicate from source path if we are set up to do this.
-        if self.duplicate and not self.has_builder() and not self.linked:
+        if self.duplicate and not self.is_derived() and not self.linked:
             src=self.srcnode().rfile()
             if src.exists() and src.abspath != self.abspath:
                 self._createDir()
@@ -1436,7 +1430,7 @@ def find_file(filename, paths, node_factory = default_fs.File):
         try:
             node = node_factory(filename, dir)
             # Return true of the node exists or is a derived node.
-            if node.has_builder() or \
+            if node.is_derived() or \
                (isinstance(node, SCons.Node.FS.Entry) and node.exists()):
                 retval = node
                 break
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 09c930b..f2d77ce 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -250,8 +250,25 @@ class Node:
         return not b is None
 
     def is_derived(self):
+        """
+        Returns true iff this node is derived (i.e. built).
+
+        This should return true only for nodes whose path should be in
+        the build directory when duplicate=0 and should contribute their build
+        signatures when they are used as source files to other derived files. For
+        example: source with source builders are not derived in this sense,
+        and hence should not return true.
+        """
         return self.has_builder() or self.side_effect
 
+    def is_pseudo_derived(self):
+        """
+        Returns true iff this node is built, but should use a source path
+        when duplicate=0 and should contribute a content signature (i.e.
+        source signature) when used as a source for other derived files.
+        """
+        return 0
+
     def alter_targets(self):
         """Return a list of alternate targets for this Node.
         """
@@ -470,7 +487,10 @@ class Node:
         or are derived.
         """
         def missing(node):
-            return not node.is_derived() and not node.linked and not node.rexists()
+            return not node.is_derived() and \
+                   not node.is_pseudo_derived() and \
+                   not node.linked and \
+                   not node.rexists()
         missing_sources = filter(missing, self.children())
         if missing_sources:
             desc = "No Builder for target `%s', needed by `%s'." % (missing_sources[0], self)
@@ -578,7 +598,7 @@ class Node:
         Return a text representation, suitable for displaying to the
         user, of the include tree for the sources of this node.
         """
-        if self.has_builder() and self.env:
+        if self.is_derived() and self.env:
             env = self.get_build_env()
             for s in self.sources:
                 def f(node, env=env, scanner=s.source_scanner, target=self):
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index b58e09f..2fbc6e2 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -255,7 +255,7 @@ class Taskmaster:
             # Find all of the derived dependencies (that is,
             # children who have builders or are side effects):
             try:
-                def derived_nodes(node): return node.is_derived()
+                def derived_nodes(node): return node.is_derived() or node.is_pseudo_derived()
                 derived = filter(derived_nodes, children)
             except:
                 # We had a problem just trying to figure out if any of
diff --git a/test/SideEffect.py b/test/SideEffect.py
index cf83b6e..f0eeee8 100644
--- a/test/SideEffect.py
+++ b/test/SideEffect.py
@@ -150,4 +150,40 @@ test.run(arguments='log')
 test.fail_test(not os.path.exists(test.workpath('log/bar.out')))
 test.fail_test(not os.path.exists(test.workpath('log/blat.out')))
 
+test.write('SConstruct', 
+"""
+def copy(source, target):
+    open(target, "wb").write(open(source, "rb").read())
+
+def build(env, source, target):
+    copy(str(source[0]), str(target[0]))
+    if target[0].side_effects:
+        side_effect = open(str(target[0].side_effects[0]), "ab")
+        side_effect.write('%s -> %s\\n'%(str(source[0]), str(target[0])))
+
+Build = Builder(action=build)
+env = Environment(BUILDERS={'Build':Build})
+Export('env')
+SConscript('SConscript', build_dir='build', duplicate=0)""")
+
+test.write('SConscript', """
+Import('env')
+env.Build('foo.out', 'foo.in')
+env.Build('bar.out', 'bar.in')
+env.Build('blat.out', 'blat.in')
+env.SideEffect('log.txt', ['foo.out', 'bar.out', 'blat.out'])
+""")
+
+test.write('foo.in', 'foo.in\n')
+test.write('bar.in', 'bar.in\n')
+
+test.run(arguments = 'build/foo.out build/bar.out')
+
+expect = """\
+foo.in -> build/foo.out
+bar.in -> build/bar.out
+"""
+
+assert test.read('build/log.txt') == expect
+
 test.pass_test()