Better error handling if someone tries to add a non-Node as a source, dependency...

git-svn-id: http://scons.tigris.org/svn/scons/trunk@875 fdb21ef1-2011-0410-befe-b5e4ea1792b1

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 474514e08c27ef41923e5c5f3b90bd3a74d28721..d567560423c75618e94cf024b78b7bf822cedfb7 100644 (file)
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -119,6 +119,9 @@ RELEASE 0.95 - XXX
     SharedLibrary() are shared work even if the object was built in a
     previous run.
 
+  - Supply meaningful error messages, not stack traces, if we try to add
+    a non-Node as a source, dependency, or ignored dependency of a Node.
+
   From Vincent Risi:
 
   - Add support for the bcc32, ilink32 and tlib Borland tools.

diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index fa0b6f75621d9725e5048ac7cb55db7e7b62b946..08e42604e1882e4f8c8360bd5e3d42f59dc1a896 100644 (file)
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -470,31 +470,39 @@ class NodeTestCase(unittest.TestCase):
         assert node.depends == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_dependency(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
 
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_dependency(zero)
+        assert node.depends == [zero]
         node.add_dependency([one])
-        assert node.depends == [one]
+        assert node.depends == [zero, one]
         node.add_dependency([two, three])
-        assert node.depends == [one, two, three]
+        assert node.depends == [zero, one, two, three]
         node.add_dependency([three, four, one])
-        assert node.depends == [one, two, three, four]
+        assert node.depends == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_depends([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.depends == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
 
     def test_add_source(self):
         """Test adding sources to a Node's list.
@@ -503,31 +511,38 @@ class NodeTestCase(unittest.TestCase):
         assert node.sources == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_source(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
-
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_source(zero)
+        assert node.sources == [zero]
         node.add_source([one])
-        assert node.sources == [one]
+        assert node.sources == [zero, one]
         node.add_source([two, three])
-        assert node.sources == [one, two, three]
+        assert node.sources == [zero, one, two, three]
         node.add_source([three, four, one])
-        assert node.sources == [one, two, three, four]
+        assert node.sources == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_source([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.sources == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
     def test_add_ignore(self):
         """Test adding files whose dependencies should be ignored.
         """
@@ -535,31 +550,38 @@ class NodeTestCase(unittest.TestCase):
         assert node.ignore == []
 
         zero = SCons.Node.Node()
-        try:
-            node.add_ignore(zero)
-        except TypeError:
-            pass
-        else:
-            assert 0
-
         one = SCons.Node.Node()
         two = SCons.Node.Node()
         three = SCons.Node.Node()
         four = SCons.Node.Node()
+        five = SCons.Node.Node()
+        six = SCons.Node.Node()
 
+        node.add_ignore(zero)
+        assert node.ignore == [zero]
         node.add_ignore([one])
-        assert node.ignore == [one]
+        assert node.ignore == [zero, one]
         node.add_ignore([two, three])
-        assert node.ignore == [one, two, three]
+        assert node.ignore == [zero, one, two, three]
         node.add_ignore([three, four, one])
-        assert node.ignore == [one, two, three, four]
+        assert node.ignore == [zero, one, two, three, four]
 
-        assert zero.get_parents() == []
+        assert zero.get_parents() == [node]
         assert one.get_parents() == [node]
         assert two.get_parents() == [node]
         assert three.get_parents() == [node]
         assert four.get_parents() == [node]
 
+        try:
+            node.add_ignore([[five, six]])
+        except:
+            pass
+        else:
+            raise "did not catch expected exception"
+        assert node.ignore == [zero, one, two, three, four]
+        assert five.get_parents() == []
+        assert six.get_parents() == []
+
     def test_get_found_includes(self):
         """Test the default get_found_includes() method
         """

diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 51bfe308f6dc0ccd95b17c1f10a560e201f6a508..28264cef3d5bc36bf0933666824f2db145ddf315 100644 (file)
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -583,22 +583,49 @@ class Node:
         return None
 
     def add_dependency(self, depend):
-        """Adds dependencies. The depend argument must be a list."""
-        self._add_child(self.depends, self.depends_dict, depend)
+        """Adds dependencies."""
+        try:
+            self._add_child(self.depends, self.depends_dict, depend)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to add a non-Node dependency to %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def add_ignore(self, depend):
-        """Adds dependencies to ignore. The depend argument must be a list."""
-        self._add_child(self.ignore, self.ignore_dict, depend)
+        """Adds dependencies to ignore."""
+        try:
+            self._add_child(self.ignore, self.ignore_dict, depend)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to ignore a non-Node dependency of %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def add_source(self, source):
-        """Adds sources. The source argument must be a list."""
-        self._add_child(self.sources, self.sources_dict, source)
+        """Adds sources."""
+        try:
+            self._add_child(self.sources, self.sources_dict, source)
+        except TypeError, e:
+            e = e.args[0]
+            if SCons.Util.is_List(e):
+                s = map(str, e)
+            else:
+                s = str(e)
+            raise SCons.Errors.UserError("attempted to add a non-Node as source of %s:\n\t%s is a %s, not a Node" % (str(self), s, type(e)))
 
     def _add_child(self, collection, dict, child):
-        """Adds 'child' to 'collection', first checking 'dict' to see if
-        it's already present. The 'child' argument must be a list"""
+        """Adds 'child' to 'collection', first checking 'dict' to see
+        if it's already present."""
         if type(child) is not type([]):
-            raise TypeError("child must be a list")
+            child = [child]
+        for c in child:
+            if not isinstance(c, Node):
+                raise TypeError, c
         added = None
         for c in child:
             if not dict.has_key(c):

diff --git a/test/Depends.py b/test/Depends.py
index 530c3c9f792b597fc33bf97e1466965882ec906a..a7d1bdf0b44773e73fcfea13166880083062cb4b 100644 (file)
--- a/test/Depends.py
+++ b/test/Depends.py
@@ -140,4 +140,18 @@ test.fail_test(test.read(['subdir', 'f4.out']) !=
 test.fail_test(test.read('f5.out') != "f5.in\nsubdir/foo.dep 3\n")
 test.fail_test(test.read(['sub2', 'f6.out']) != "f6.in 3\nsubdir/bar.dep 3\n")
 
+#
+test.write('SConstruct', """\
+env = Environment()
+file1 = File('file1')
+file2 = File('file2')
+env.Depends(file1, [[file2, 'file3']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to add a non-Node dependency to file1:
+\t['file2', 'file3'] is a <type 'list'>, not a Node
+File "SConstruct", line 4, in ?
+""")
+
 test.pass_test()

diff --git a/test/Ignore.py b/test/Ignore.py
index 5e9650e925d7465a21fb4d36d5d38039a5d4804e..06db727737b235348bdee3185706dd248d7e29cd 100644 (file)
--- a/test/Ignore.py
+++ b/test/Ignore.py
@@ -108,4 +108,18 @@ test.fail_test(test.read(['subdir', 'f3.out']) !=
 
 test.up_to_date(arguments = '.')
 
+#
+test.write('SConstruct', """\
+env = Environment()
+file1 = File('file1')
+file2 = File('file2')
+env.Ignore(file1, [[file2, 'file3']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to ignore a non-Node dependency of file1:
+\t['file2', 'file3'] is a <type 'list'>, not a Node
+File "SConstruct", line 4, in ?
+""")
+
 test.pass_test()

diff --git a/test/Program.py b/test/Program.py
index f231ac1f75ac5f19ff9ee36e3800176a84615c7e..7c0eb462fc2b28fb4692499b5c48d6f0387171f4 100644 (file)
--- a/test/Program.py
+++ b/test/Program.py
@@ -327,4 +327,17 @@ test.fail_test(not (oldtime3 == os.path.getmtime(foo3)))
 test.fail_test(not (oldtime4 == os.path.getmtime(foo4)))
 test.fail_test(not (oldtime5 == os.path.getmtime(foo5)))
 
+# 
+test.write('SConstruct', """\
+file1 = File('file1.c')
+file2 = File('file2.c')
+Program('foo', [file1, [file2, 'file3.c']])
+""")
+
+test.run(status = 2, stderr = """
+scons: *** attempted to add a non-Node as source of foo:
+\t['file2.c', 'file3.c'] is a <type 'list'>, not a Node
+File "SConstruct", line 3, in ?
+""")
+
 test.pass_test()