Better error messages. (Gary Oberbrunner)

5 files changed, 87 insertions, 13 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index bd8c5da..40fdbbb 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -22,6 +22,16 @@ RELEASE 0.XX - XXX
 
   - Documentation fixes:  typo in the man page.
 
+  From Gary Oberbrunner:
+
+  - Report the target being built in error messages when building
+    multiple sources from different extensions, or when the target file
+    extension can't be deduced, or when we don't have an action for a
+    file suffix.
+
+  - Provide helpful error messages when the arguments to env.Install()
+    are incorrect.
+
 
 
 RELEASE 0.90 - Wed, 25 Jun 2003 14:24:52 -0500
diff --git a/src/engine/SCons/Builder.py b/src/engine/SCons/Builder.py
index f19983d..2d5828a 100644
--- a/src/engine/SCons/Builder.py
+++ b/src/engine/SCons/Builder.py
@@ -83,17 +83,17 @@ class DictCmdGenerator:
         for src in map(str, source):
             my_ext = SCons.Util.splitext(src)[1]
             if ext and my_ext != ext:
-                raise UserError("Cannot build multiple sources with different extensions: %s, %s" % (ext, my_ext))
+                raise UserError("While building `%s' from `%s': Cannot build multiple sources with different extensions: %s, %s" % (repr(map(str, target)), src, ext, my_ext))
             ext = my_ext
 
-        if ext is None:
-            raise UserError("Cannot deduce file extension from source files: %s" % repr(map(str, source)))
+        if not ext:
+            raise UserError("While building `%s': Cannot deduce file extension from source files: %s" % (repr(map(str, target)), repr(map(str, source))))
         try:
             # XXX Do we need to perform Environment substitution
             # on the keys of action_dict before looking it up?
             return self.action_dict[ext]
         except KeyError:
-            raise UserError("Don't know how to build a file with suffix %s." % ext)
+            raise UserError("While building `%s': Don't know how to build a file with suffix %s." % (repr(map(str, target)), repr(ext)))
     def __cmp__(self, other):
         return cmp(self.action_dict, other.action_dict)
 
diff --git a/src/engine/SCons/BuilderTests.py b/src/engine/SCons/BuilderTests.py
index 062f8b9..d486e70 100644
--- a/src/engine/SCons/BuilderTests.py
+++ b/src/engine/SCons/BuilderTests.py
@@ -459,9 +459,11 @@ class BuilderTestCase(unittest.TestCase):
         tgt = builder(env, target='test3', source=['test2.bar', 'test1.foo'])
         try:
             tgt.build()
-        except SCons.Errors.UserError:
+        except SCons.Errors.UserError, e:
             flag = 1
         assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['test3']' from `test1.foo': Cannot build multiple sources with different extensions: .bar, .foo"
+        assert match, e
 
         foo_bld = SCons.Builder.Builder(action = 'a-foo',
                                         src_suffix = '.ina',
@@ -497,28 +499,54 @@ class BuilderTestCase(unittest.TestCase):
         assert isinstance(tgt.builder, SCons.Builder.MultiStepBuilder)
 
         flag = 0
-        tgt = builder(env, target='t5', source='test5a.foo test5b.inb')
+        tgt = builder(env, target='t5', source=['test5a.foo', 'test5b.inb'])
         try:
             tgt.build()
-        except SCons.Errors.UserError:
+        except SCons.Errors.UserError, e:
             flag = 1
         assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['t5']' from `test5b.bar': Cannot build multiple sources with different extensions: .foo, .bar"
+        assert match, e
 
         flag = 0
-        tgt = builder(env, target='t6', source='test6a.bar test6b.ina')
+        tgt = builder(env, target='t6', source=['test6a.bar', 'test6b.ina'])
         try:
             tgt.build()
-        except SCons.Errors.UserError:
+        except SCons.Errors.UserError, e:
             flag = 1
         assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['t6']' from `test6b.foo': Cannot build multiple sources with different extensions: .bar, .foo"
+        assert match, e
 
         flag = 0
-        tgt = builder(env, target='t4', source='test4a.ina test4b.inb')
+        tgt = builder(env, target='t4', source=['test4a.ina', 'test4b.inb'])
         try:
             tgt.build()
-        except SCons.Errors.UserError:
+        except SCons.Errors.UserError, e:
             flag = 1
         assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['t4']' from `test4b.bar': Cannot build multiple sources with different extensions: .foo, .bar"
+        assert match, e
+
+        flag = 0
+        tgt = builder(env, target='t7', source=['test7'])
+        try:
+            tgt.build()
+        except SCons.Errors.UserError, e:
+            flag = 1
+        assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['t7']': Cannot deduce file extension from source files: ['test7']"
+        assert match, e
+
+        flag = 0
+        tgt = builder(env, target='t8', source=['test8.unknown'])
+        try:
+            tgt.build()
+        except SCons.Errors.UserError, e:
+            flag = 1
+        assert flag, "UserError should be thrown when we build targets with files of different suffixes."
+        match = str(e) == "While building `['t8']': Don't know how to build a file with suffix '.unknown'."
+        assert match, e
 
     def test_build_scanner(self):
         """Testing ability to set a target scanner through a builder."""
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 04452a9..cbf7a7c 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -406,8 +406,17 @@ class Environment:
 
     def Install(self, dir, source):
         """Install specified files in the given directory."""
-        sources = SCons.Node.arg2nodes(source, self.fs.File)
-        dnodes = SCons.Node.arg2nodes(dir, self.fs.Dir)
+        try:
+            dnodes = SCons.Node.arg2nodes(dir, self.fs.Dir)
+        except TypeError:
+            raise SCons.Errors.UserError, "Target `%s' of Install() is a file, but should be a directory.  Perhaps you have the Install() arguments backwards?" % str(dir)
+        try:
+            sources = SCons.Node.arg2nodes(source, self.fs.File)
+        except TypeError:
+            if SCons.Util.is_List(source):
+                raise SCons.Errors.UserError, "Source `%s' of Install() contains one or more non-files.  Install() source must be one or more files." % repr(map(str, source))
+            else:
+                raise SCons.Errors.UserError, "Source `%s' of Install() is not a file.  Install() source must be one or more files." % str(source)
         tgt = []
         for dnode in dnodes:
             for src in sources:
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index 3da8334..080c0d9 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -344,6 +344,33 @@ class EnvironmentTestCase(unittest.TestCase):
         for tnode in tgt:
             assert tnode.builder == InstallBuilder
 
+        exc_caught = None
+        try:
+            tgt = env.Install('export', 'export')
+        except SCons.Errors.UserError, e:
+            exc_caught = 1
+        assert exc_caught, "UserError should be thrown when Install() target is not a file."
+        match = str(e) == "Source `export' of Install() is not a file.  Install() source must be one or more files."
+        assert match, e
+
+        exc_caught = None
+        try:
+            tgt = env.Install('export', ['export', 'build/foo1'])
+        except SCons.Errors.UserError, e:
+            exc_caught = 1
+        assert exc_caught, "UserError should be thrown when Install() target containins non-files."
+        match = str(e) == "Source `['export', 'build/foo1']' of Install() contains one or more non-files.  Install() source must be one or more files."
+        assert match, e
+
+        exc_caught = None
+        try:
+            tgt = env.Install('export/foo1', 'build/foo1')
+        except SCons.Errors.UserError, e:
+            exc_caught = 1
+        assert exc_caught, "UserError should be thrown reversing the order of Install() targets."
+        match = str(e) == "Target `export/foo1' of Install() is a file, but should be a directory.  Perhaps you have the Install() arguments backwards?"
+        assert match, e
+
         tgt = env.InstallAs(target=string.split('foo1 foo2'),
                             source=string.split('bar1 bar2'))
         assert len(tgt) == 2, len(tgt)