Merge branch 'master' of github.com:breed/scons

4 files changed, 88 insertions, 7 deletions

diff --git a/src/engine/SCons/CacheDir.py b/src/engine/SCons/CacheDir.py
index 704b9a5..10c088d 100644
--- a/src/engine/SCons/CacheDir.py
+++ b/src/engine/SCons/CacheDir.py
@@ -33,6 +33,7 @@ import os
 import stat
 import sys
 
+import SCons
 import SCons.Action
 import SCons.Warnings
 from SCons.Util import PY3
@@ -185,7 +186,7 @@ class CacheDir(object):
             pass
         except OSError:
             msg = "Failed to create cache directory " + path
-            raise SCons.Errors.EnvironmentError(msg)
+            raise SCons.Errors.SConsEnvironmentError(msg)
 
         try:
             with open(config_file, 'x') as config:
@@ -194,14 +195,14 @@ class CacheDir(object):
                     json.dump(self.config, config)
                 except Exception:
                     msg = "Failed to write cache configuration for " + path
-                    raise SCons.Errors.EnvironmentError(msg)
+                    raise SCons.Errors.SConsEnvironmentError(msg)
         except FileExistsError:
             try:
                 with open(config_file) as config:
                     self.config = json.load(config)
             except ValueError:
                 msg = "Failed to read cache configuration for " + path
-                raise SCons.Errors.EnvironmentError(msg)
+                raise SCons.Errors.SConsEnvironmentError(msg)
 
 
     def _readconfig2(self, path):
diff --git a/src/engine/SCons/CacheDirTests.py b/src/engine/SCons/CacheDirTests.py
index ef87746..07c32b4 100644
--- a/src/engine/SCons/CacheDirTests.py
+++ b/src/engine/SCons/CacheDirTests.py
@@ -27,10 +27,13 @@ import os.path
 import shutil
 import sys
 import unittest
+import tempfile
+import stat
 
 from TestCmd import TestCmd
 
 import SCons.CacheDir
+from SCons.Util import PY3
 
 built_it = None
 
@@ -112,6 +115,84 @@ class CacheDirTestCase(BaseTestCase):
         finally:
             SCons.Util.MD5collect = save_collect
 
+class ExceptionTestCase(unittest.TestCase):
+    """Test that the correct exceptions are thrown by CacheDir."""
+
+    # Don't inherit from BaseTestCase, we're by definition trying to
+    # break things so we really want a clean slate for each test.
+    def setUp(self):
+        self.tmpdir = tempfile.mkdtemp()
+        self._CacheDir = SCons.CacheDir.CacheDir(self.tmpdir)
+
+    def tearDown(self):
+        shutil.rmtree(self.tmpdir)
+
+    @unittest.skipIf(sys.platform.startswith("win"), "This fixture will not trigger an OSError on Windows")
+    def test_throws_correct_on_OSError(self):
+        """Test that the correct error is thrown when cache directory cannot be created."""
+        privileged_dir = os.path.join(os.getcwd(), "privileged")
+        try:
+            os.mkdir(privileged_dir)
+            os.chmod(privileged_dir, stat.S_IREAD)
+            cd = SCons.CacheDir.CacheDir(os.path.join(privileged_dir, "cache"))
+            assert False, "Should have raised exception and did not"
+        except SCons.Errors.SConsEnvironmentError as e:
+            assert str(e) == "Failed to create cache directory {}".format(os.path.join(privileged_dir, "cache"))
+        finally:
+            os.chmod(privileged_dir, stat.S_IWRITE | stat.S_IEXEC | stat.S_IREAD)
+            shutil.rmtree(privileged_dir)
+
+
+    def test_throws_correct_when_failed_to_write_configfile(self):
+        class Unserializable:
+            """A class which the JSON should not be able to serialize"""
+
+            def __init__(self, oldconfig):
+                self.something = 1 # Make the object unserializable
+                # Pretend to be the old config just enough
+                self.__dict__["prefix_len"] = oldconfig["prefix_len"]
+
+            def __getitem__(self, name, default=None):
+                if name == "prefix_len":
+                    return self.__dict__["prefix_len"]
+                else:
+                    return None
+
+            def __setitem__(self, name, value):
+                self.__dict__[name] = value
+
+        oldconfig = self._CacheDir.config
+        self._CacheDir.config = Unserializable(oldconfig)
+        # Remove the config file that got created on object creation
+        # so that _readconfig* will try to rewrite it
+        old_config = os.path.join(self._CacheDir.path, "config")
+        os.remove(old_config)
+        
+        try:
+            if PY3:
+                self._CacheDir._readconfig3(self._CacheDir.path)
+            else:
+                self._CacheDir._readconfig2(self._CacheDir.path)
+            assert False, "Should have raised exception and did not"
+        except SCons.Errors.SConsEnvironmentError as e:
+            assert str(e) == "Failed to write cache configuration for {}".format(self._CacheDir.path)
+
+    def test_raise_environment_error_on_invalid_json(self):
+        config_file = os.path.join(self._CacheDir.path, "config")
+        with open(config_file, "r") as cfg:
+            content = cfg.read()
+        # This will make JSON load raise a ValueError
+        content += "{}"
+        with open(config_file, "w") as cfg:
+            cfg.write(content)
+
+        try:
+            # Construct a new cache dir that will try to read the invalid config
+            new_cache_dir = SCons.CacheDir.CacheDir(self._CacheDir.path)
+            assert False, "Should have raised exception and did not"
+        except SCons.Errors.SConsEnvironmentError as e:
+            assert str(e) == "Failed to read cache configuration for {}".format(self._CacheDir.path)
+
 class FileTestCase(BaseTestCase):
     """
     Test calling CacheDir code through Node.FS.File interfaces.
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 33105fb..6b0fe98 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -3436,6 +3436,8 @@ class File(Base):
             Boolean - Indicates if node(File) has changed.
         """
 
+        if node is None:
+            node = self
         # Now get sconsign name -> csig map and then get proper prev_ni if possible
         bi = node.get_stored_info().binfo
         rebuilt = False
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 3073d59..572465f 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -1661,10 +1661,7 @@ class Node(object, with_metaclass(NoSlotsPyPy)):
             if k not in old_bkids:
                 lines.append("`%s' is a new dependency\n" % stringify(k))
             else:
-                try:
-                    changed = _decider_map[k.changed_since_last_build](k, self, osig[k])
-                except DeciderNeedsNode as e:
-                    changed = e.decider(self, osig[k], node=self)
+                changed = _decider_map[k.changed_since_last_build](k, self, osig[k])
 
                 if changed:
                     lines.append("`%s' changed\n" % stringify(k))