Fix the use of reflective paths underneath build directories, when the path under the build_dir matches the path to the build_dir. (Kevin Quick)

4 files changed, 208 insertions, 1 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 5f6968b..ded0c5c 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -264,6 +264,10 @@ RELEASE 0.97 - XXX
   - Python Function actions now have their calling signature (target,
     source, env) reported correctly when displayed.
 
+  - Fix BuildDir()/build_dir handling when the build_dir is underneath
+    the source directory and trying to use entries from the build_dir
+    as sources for other targets in the build-dir.
+
   From Levi Stephen:
 
   - Allow $JARCHDIR to be expanded to other construction variables.
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 49cbe7d..5ee4abf 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -541,6 +541,11 @@ class Base(SCons.Node.Node):
                 if dir.srcdir:
                     self._srcnode = self.fs.Entry(name, dir.srcdir,
                                                   klass=self.__class__)
+                    if self._srcnode.is_under(dir):
+                        # Shouldn't source from something in the build
+                        # path: probably means build_dir is under
+                        # src_dir and we are reflecting.
+                        break
                     return self._srcnode
                 name = dir.name + os.sep + name
                 dir=dir.get_dir()
@@ -1101,8 +1106,17 @@ class FS(LocalFS):
         """
         targets = []
         message = None
+        start_dir = dir
+        start_tail = tail[:]
         while dir:
             for bd in dir.build_dirs:
+                if start_dir.is_under(bd):
+                    # If already in the build-dir location, don't reflect
+                    e = start_dir
+                    if start_tail:
+                        e = e.Entry(start_tail[0])
+                    targets.append(e)
+                    continue
                 p = apply(os.path.join, [bd.path] + tail)
                 targets.append(self.Entry(p))
             tail = [dir.name] + tail
@@ -1669,7 +1683,7 @@ class File(Base):
         # Duplicate from source path if we are set up to do this.
         if self.duplicate and not self.is_derived() and not self.linked:
             src=self.srcnode().rfile()
-            if src.exists() and src.abspath != self.abspath:
+            if src.abspath != self.abspath and src.exists():
                 self._createDir()
                 try:
                     Unlink(self, None, None)
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index c47435f..4031d3f 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -521,6 +521,73 @@ class BuildDirTestCase(unittest.TestCase):
                     delattr(os, 'symlink')
                 shutil.copy2 = real_copy
 
+        # Test BuildDir "reflection," where a same-named subdirectory
+        # exists underneath a build_dir.
+        fs = SCons.Node.FS.FS()
+        fs.BuildDir('work/src/b1/b2', 'work/src')
+
+        dir_list = [
+                'work/src',
+                'work/src/b1',
+                'work/src/b1/b2',
+                'work/src/b1/b2/b1',
+                'work/src/b1/b2/b1/b2',
+        ]
+
+        srcnode_map = {
+                'work/src/b1/b2' : 'work/src',
+                'work/src/b1/b2/f' : 'work/src/f',
+                'work/src/b1/b2/b1' : 'work/src/b1/',
+                'work/src/b1/b2/b1/f' : 'work/src/b1/f',
+        }
+
+        alter_map = {
+                'work/src' : 'work/src/b1/b2',
+                'work/src/f' : 'work/src/b1/b2/f',
+                'work/src/b1' : 'work/src/b1/b2/b1',
+                'work/src/b1/f' : 'work/src/b1/b2/b1/f',
+        }
+
+        errors = 0
+
+        for dir in dir_list:
+            dnode = fs.Dir(dir)
+            f = dir + '/f'
+            fnode = fs.File(dir + '/f')
+
+            dp = dnode.srcnode().path
+            expect = os.path.normpath(srcnode_map.get(dir, dir))
+            if dp != expect:
+                print "Dir `%s' srcnode() `%s' != expected `%s'" % (dir, dp, expect)
+                errors = errors + 1
+
+            fp = fnode.srcnode().path
+            expect = os.path.normpath(srcnode_map.get(f, f))
+            if fp != expect:
+                print "File `%s' srcnode() `%s' != expected `%s'" % (f, fp, expect)
+                errors = errors + 1
+
+        for dir in dir_list:
+            dnode = fs.Dir(dir)
+            f = dir + '/f'
+            fnode = fs.File(dir + '/f')
+
+            t, m = dnode.alter_targets()
+            tp = t[0].path
+            expect = os.path.normpath(alter_map.get(dir, dir))
+            if tp != expect:
+                print "Dir `%s' alter_targets() `%s' != expected `%s'" % (dir, tp, expect)
+                errors = errors + 1
+
+            t, m = fnode.alter_targets()
+            tp = t[0].path
+            expect = os.path.normpath(alter_map.get(f, f))
+            if tp != expect:
+                print "File `%s' alter_targets() `%s' != expected `%s'" % (f, tp, expect)
+                errors = errors + 1
+
+        self.failIf(errors)
+
 class FSTestCase(unittest.TestCase):
     def runTest(self):
         """Test FS (file system) Node operations
diff --git a/test/builddir-reflect.py b/test/builddir-reflect.py
new file mode 100644
index 0000000..0adc45e
--- /dev/null
+++ b/test/builddir-reflect.py
@@ -0,0 +1,122 @@
+#!/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__"
+
+"""
+This test validates the correct operation of a BuildDir specification
+in avoiding reflection: reflection is the case where the build_dir is
+located under the corresponding source dir, and trying to use elements
+in the build_dir as sources for that same build dir.
+
+Test based on bug #1055521 filed by Gary Oberbrunner.
+"""
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+python = TestSCons.python
+
+test.write("mycc.py", """
+print 'Compile'
+""")
+
+test.write("mylink.py", """
+print 'Link'
+""")
+
+sconstruct = """
+env = Environment(CC = r'%(python)s mycc.py',
+                  LINK = r'%(python)s mylink.py',
+                  INCPREFIX = 'INC_',
+                  INCSUFFIX = '_CNI',
+                  CPPPATH='%(cpppath)s')  # note no leading '#'
+Export("env")
+SConscript('SConscript', build_dir="dir1/dir2", src_dir=".")
+"""
+
+test.write('SConscript', """\
+Import("env")
+env.Program("foo", "src1/foo.c")
+Default(".")
+""")
+
+test.write('foo.h', '#define HI_STR "hello, there!"\n')
+
+test.subdir('src1')
+
+test.write(['src1', 'foo.c'], """\
+#include <stdio.h>
+#include "foo.h"
+main() { printf(HI_STR);}
+""")
+
+# Test the bad cpppath; make sure it doesn't reflect dir1/dir2/foo.h
+# into dir1/dir2/dir1/dir2/foo.h, and make sure the target/message for
+# builds is correct.
+
+cpppath = 'dir1/dir2'   # note, no leading '#'
+test.write('SConstruct', sconstruct % locals() )
+
+test.run(arguments = '',
+         stdout=test.wrap_stdout("""\
+scons: building associated BuildDir targets: dir1/dir2
+%(python)s mycc.py INC_dir1/dir2/dir1/dir2_CNI .+
+Compile
+%(python)s mylink.py .+
+Link
+""" % locals()),
+         match=TestSCons.match_re,
+         )
+
+test.must_exist(['dir1', 'dir2', 'foo.h'])
+test.must_exist(['dir1', 'dir2', 'src1', 'foo.c'])
+test.must_not_exist(['dir1', 'dir2', 'dir1', 'dir2', 'foo.h'])
+
+import shutil
+shutil.rmtree('dir1', ignore_errors=1)
+test.must_not_exist('dir1')
+
+# Now test the good cpppath and make sure everything looks right.
+
+cpppath = '#dir1/dir2'   # note leading '#'
+test.write('SConstruct', sconstruct % locals() )
+
+test.run(arguments = '',
+         stdout=test.wrap_stdout("""\
+scons: building associated BuildDir targets: dir1/dir2
+%(python)s mycc.py INC_dir1/dir2_CNI .+
+Compile
+%(python)s mylink.py .+
+Link
+""" % locals()),
+         match=TestSCons.match_re,
+         )
+
+test.must_exist(['dir1', 'dir2', 'foo.h'])
+test.must_exist(['dir1', 'dir2', 'src1', 'foo.c'])
+test.must_not_exist(['dir1', 'dir2', 'dir1', 'dir2', 'foo.h'])
+
+
+test.pass_test()