Merge branch 'master' into master

7 files changed, 166 insertions, 21 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 85eb87267..e33aa6a15 100755
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -6,13 +6,23 @@
 
 RELEASE  VERSION/DATE TO BE FILLED IN LATER
 
-  From John Doe:
+  From William Deegan:
+    - Remove obsoleted references to DeciderNeedsNode which could cause crash when using --debug=explain
 
-    - Whatever John Doe did.
+  From Jason Kenny
+    - Add Fix and test for regression in 3.1.0 when using Decider('MD5-timestamp') and --debug=explain
 
   From Ben Reed:
     - Added -fmerge-all-constants to flags that get included in both CCFLAGS and LINKFLAGS.
 
+  From Mathew Robinson:
+    - Fix issue #3415 - Update remaining usages of EnvironmentError to SConsEnvironmentError
+      this patch fixes issues introduced in 3.1.0 where any of the
+      following would cause SCons to error and exit:
+        - CacheDir not write-able
+        - JSON encoding errors for CacheDir config
+        - JSON decoding errors for CacheDir config
+
 
 RELEASE 3.1.0 - Mon, 20 Jul 2019 16:59:23 -0700
 
@@ -23,7 +33,7 @@ RELEASE 3.1.0 - Mon, 20 Jul 2019 16:59:23 -0700
   From William Deegan:
     - Enhanced --debug=explain output. Now the separate components of the dependency list are split up
       as follows:
-      
+
       scons: rebuilding `file3' because:
            the dependency order changed:
            ->Sources
@@ -94,7 +104,7 @@ RELEASE 3.1.0 - Mon, 20 Jul 2019 16:59:23 -0700
     - More fixes for newer Java versions (since 9): handle new jdk directory
       naming (jdk-X.Y instead of jdkX.Y) on Windows; handle two-digit major
       version. Docstrings improved.
-    - Fixups for pylint: exception types, redefined functions, 
+    - Fixups for pylint: exception types, redefined functions,
       globals, etc.  Some old code removed to resolve issues (hashlib is
       always present on modern Pythons; no longer need the code for
       2.5-and-earlier optparse). cmp is not a builtin function in Py3,
@@ -104,7 +114,7 @@ RELEASE 3.1.0 - Mon, 20 Jul 2019 16:59:23 -0700
     - Add a PY3-only function for setting up the cachedir that should be less
       prone to races. Add a hack to the PY2 version (from Issue #3351) to
       be less prone to a race in the check for old-style cache.
-    - Fix coding error in docbook tool only exercised when using python lxml 
+    - Fix coding error in docbook tool only exercised when using python lxml
     - Recognize two additional GNU compiler header directory options in
       ParseFlags: -iquote and -idirafter.
     - Fix more re patterns that contain \ but not specified as raw strings
@@ -151,7 +161,7 @@ From Daniel Moody:
 
   From Bernhard M. Wiedemann:
     - Do not store build host+user name if reproducible builds are wanted
-    
+
 
 RELEASE 3.0.4 - Mon, 20 Jan 2019 22:49:27 +0000
 
@@ -219,9 +229,9 @@ RELEASE 3.0.2 - Mon, 31 Dec 2018 16:00:12 -0700
     - Fix GH Issue #2580 - # in FRAMEWORKPATH doesn't get properly expanded. The # is left in the
       command line.
     - Fix issue #2980 with credit to Piotr Bartosik (and William Blevins).  This is an issue where using
-      TimeStamp-MD5 Decider and CacheDir can yield incorrect md5's being written into the .sconsign.  
-      The difference between Piotr Bartosik's patch and the current code is that the more complicated 
-      creation of file to csig map is only done when the count of children for the current node doesn't 
+      TimeStamp-MD5 Decider and CacheDir can yield incorrect md5's being written into the .sconsign.
+      The difference between Piotr Bartosik's patch and the current code is that the more complicated
+      creation of file to csig map is only done when the count of children for the current node doesn't
       match the previous count which is loaded from the sconsign.
     - Fix issue # 3106 MSVC if using MSVC_BATCH and target dir had a space would fail due to quirk in
       MSVC's handling of escaped targetdirs when batch compiling.
@@ -257,7 +267,7 @@ RELEASE 3.0.2 - Mon, 31 Dec 2018 16:00:12 -0700
     - Removed unused --warn options from the man page and source code.
 
   From Arda Fu
-    - Fix cpp scanner regex logic to treat ifndef for py3.5+. Previously it was 
+    - Fix cpp scanner regex logic to treat ifndef for py3.5+. Previously it was
       not properly differentiating between if, ifdef, and ifndef.
 
   From Philipp Maierhöfer
@@ -268,7 +278,7 @@ RELEASE 3.0.2 - Mon, 31 Dec 2018 16:00:12 -0700
   From Matthew Marinets:
     - Fixed an issue that caused the Java emitter to incorrectly parse arguments to constructors that
       implemented a class.
-      
+
   From Fredrik Medley:
     - Fix exception when printing of EnviromentError messages.
       Specifically, this fixes error reporting of the race condition when
@@ -389,7 +399,7 @@ RELEASE 3.0.2 - Mon, 31 Dec 2018 16:00:12 -0700
       filter type -> list in ipk
 
   From Bernhard M. Wiedemann:
-    - Update SCons' internal scons build logic to allow overriding build date 
+    - Update SCons' internal scons build logic to allow overriding build date
       with SOURCE_DATE_EPOCH for SCons itself.
     - Change the datestamps in SCons' docs and embedded in code use ISO 8601 format and UTC
 
diff --git a/src/engine/SCons/CacheDir.py b/src/engine/SCons/CacheDir.py
index 704b9a535..10c088da3 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 ef87746c6..07c32b409 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 33105fb66..6b0fe982f 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 3073d5910..572465f29 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))
diff --git a/test/Configure/option--config.py b/test/Configure/option--config.py
index 02f10cee5..ad4f1447f 100644
--- a/test/Configure/option--config.py
+++ b/test/Configure/option--config.py
@@ -126,8 +126,6 @@ test.checkLogAndStdout(["Checking for C header file non_system_header0.h... ",
 test.file_fixture('test_main.c')
 
 # Check the combination of --config=force and Decider('MD5-timestamp')
-# On second run there was an issue where the decider would throw DeciderNeedsNode
-# exception which the configure code didn't handle.
 SConstruct_path = test.workpath('SConstruct')
 test.write(SConstruct_path, """
 env = Environment()
diff --git a/test/Decider/MD5-timestamp-explain.py b/test/Decider/MD5-timestamp-explain.py
new file mode 100644
index 000000000..357d92492
--- /dev/null
+++ b/test/Decider/MD5-timestamp-explain.py
@@ -0,0 +1,56 @@
+#!/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__"
+
+"""
+Verify behavior of the MD5-timestamp Decider() setting.
+"""
+
+import TestSCons
+
+test = TestSCons.TestSCons()
+
+test.write('SConstruct', """\
+import random
+env = Environment()
+env.Decider('MD5-timestamp')
+# for testing use raw python API
+with open("hello.h.in","w") as outfile:
+    outfile.write("// {}".format(random.randint(1,100)))
+env.Command("hello.h","hello.h.in",[Copy("$TARGET","$SOURCE")])
+""")
+
+
+test.run(arguments = '--debug=explain')
+# this should not be up-to-date and it should not crash
+test.not_up_to_date(arguments = '--debug=explain')
+
+test.pass_test()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4: