improved DoubleCacheDir test, added extra cachedir validation check.

6 files changed, 63 insertions, 19 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 186a2cf..aff8b19 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -13,7 +13,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       It will now allow callables with expected args, and any extra args as long as they
       have default arguments. Additionally functions with no defaults for extra arguments
       as long as they are set using functools.partial to create a new callable which set them.
-    - Fix Issue #3035 - mingw with SHLIBVERSION set fails with either not a dll error or 
+    - Fix Issue #3035 - mingw with SHLIBVERSION set fails with either not a dll error or
       "Multiple ways to build the same target were specified for:".  Now mingw will disable
       creating the symlinks (and adding version string to ) dlls.  It sets SHLIBNOVERSIONSYMLINKS,
       IMPLIBNOVERSIONSYMLINKS and LDMODULENOVERSIONSYMLINKS to True.
@@ -27,9 +27,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Update CacheDir to use uuid for tmpfile uniqueness instead of pid.
       This fixes cases for shared cache where two systems write to the same
       cache tmpfile at the same time because the happened to get the same pid.
-    - Added support for passing custom CacheDir derived classes to SCons to
-      to support MongoDB's use of a compressed CacheDir tool. Moved copy_from_cache
-      attribute from the Environment class to CacheDir class.
+    - Added support for passing custom CacheDir derived classes to SCons. Moved
+      copy_from_cache attribute from the Environment class to CacheDir class.
+      Code contributed by MongoDB.
 
   From Mats Wichmann:
     - Initial support in tests for Python 3.10 - expected bytecode and
diff --git a/RELEASE.txt b/RELEASE.txt
index a73637d..840da0a 100755
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -39,7 +39,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
       WIN32DEFPREFIX, WIN32DEFSUFFIX, WIN32EXPPREFIX, WIN32EXPSUFFIX.
       All have been replaced by other names since at least 1.0.
 
-- Moved copy_from_cache attribute from the Environment class to CacheDir class.
+    - Added CACHEDIR_CLASS construction variable and expanded CacheDir method args
+      to support SCons invoking CacheDir derived classes for CacheDir customization.
+      Moved copy_from_cache attribute from the Environment class to CacheDir class.
+      Code contributed by MongoDB.
 
 FIXES
 -----
diff --git a/SCons/Environment.py b/SCons/Environment.py
index 35b5d3c..eefd8da 100644
--- a/SCons/Environment.py
+++ b/SCons/Environment.py
@@ -1031,22 +1031,31 @@ class Base(SubstitutionEnvironment):
         except KeyError:
             return None
 
+    def validate_CacheDir_class(self, custom_class=None):
+        """Validate the passed custom CacheDir class, or if no args are passed,
+        validate the custom CacheDir class from the environment.
+        """
+
+        if custom_class is None:
+            custom_class = self.get("CACHEDIR_CLASS", SCons.CacheDir.CacheDir)
+        if not issubclass(custom_class, SCons.CacheDir.CacheDir):
+            raise UserError("Custom CACHEDIR_CLASS %s not derived from CacheDir" % str(custom_class))
+        return custom_class
+
     def get_CacheDir(self):
         try:
             path = self._CacheDir_path
         except AttributeError:
             path = SCons.Defaults.DefaultEnvironment()._CacheDir_path
+
+        cachedir_class = self.validate_CacheDir_class()
         try:
             if (path == self._last_CacheDir_path
-                    and self.get("CACHEDIR_CLASS", SCons.CacheDir.CacheDir) == type(self._last_CacheDir)):
+                    and cachedir_class is type(self._last_CacheDir)):
                 return self._last_CacheDir
         except AttributeError:
             pass
 
-        cachedir_class = self.get("CACHEDIR_CLASS", SCons.CacheDir.CacheDir)
-        if not issubclass(cachedir_class, SCons.CacheDir.CacheDir):
-            raise UserError("Custom CACHEDIR_CLASS %s not derived from CacheDir" % str(cachedir_class))
-
         cd = cachedir_class(path)
         self._last_CacheDir_path = path
         self._last_CacheDir = cd
@@ -1994,7 +2003,7 @@ class Base(SubstitutionEnvironment):
         self._CacheDir_path = path
 
         if custom_class:
-            self['CACHEDIR_CLASS'] = custom_class
+            self['CACHEDIR_CLASS'] = self.validate_CacheDir_class(custom_class)
 
         if SCons.Action.execute_actions:
             # Only initialize the CacheDir if  -n/-no_exec was NOT specified.
diff --git a/test/CacheDir/DoubleCachedirClass.py b/test/CacheDir/DoubleCachedirClass.py
index 3d06f7f..af97ec2 100644
--- a/test/CacheDir/DoubleCachedirClass.py
+++ b/test/CacheDir/DoubleCachedirClass.py
@@ -25,7 +25,7 @@
 #
 
 """
-Test that a custom cache dir can be passed to SCons.
+Test that a custom cache dir is not incorrectly duplicated.
 """
 
 import TestSCons
@@ -36,7 +36,9 @@ test.dir_fixture('double_cachedir_fixture')
 
 test.run()
 
-test.must_contain_all_lines(test.stdout(), ["MY_CUSTOM_CACHEDIR_CLASS2"])
+test.must_contain_single_instance_of(test.stdout(), ["INSTANCIATED CustomCacheDir2"])
+test.must_contain_single_instance_of(test.stdout(), ["MY_CUSTOM_CACHEDIR_CLASS2"])
+test.must_not_contain_any_line(test.stdout(), ["MY_CUSTOM_CACHEDIR_CLASS1"])
 
 test.pass_test()
 
diff --git a/test/CacheDir/double_cachedir_fixture/SConstruct b/test/CacheDir/double_cachedir_fixture/SConstruct
index 1ec442b..bcb39c5 100644
--- a/test/CacheDir/double_cachedir_fixture/SConstruct
+++ b/test/CacheDir/double_cachedir_fixture/SConstruct
@@ -3,7 +3,7 @@ class CustomCacheDir1(SCons.CacheDir.CacheDir):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        print("INSTANCIATED %s" % str(type(self)))
+        print("INSTANCIATED %s" % str(type(self).__name__))
 
     @classmethod
     def copy_to_cache(cls, env, src, dst):
@@ -14,7 +14,7 @@ class CustomCacheDir2(SCons.CacheDir.CacheDir):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        print("INSTANCIATED %s" % str(type(self)))
+        print("INSTANCIATED %s" % str(type(self).__name__))
 
     @classmethod
     def copy_to_cache(cls, env, src, dst):
@@ -23,8 +23,10 @@ class CustomCacheDir2(SCons.CacheDir.CacheDir):
 
 env = Environment(tools=[])
 env.CacheDir('cache1', CustomCacheDir1)
-env.Command('file.out', 'file.in', Copy('$TARGET', '$SOURCE'))
 env.CacheDir('cache2', CustomCacheDir2)
 
+cloned = env.Clone()
+cloned.Command('file.out', 'file.in', Copy('$TARGET', '$SOURCE'))
 
+env.CacheDir('cache1', CustomCacheDir1)
 
diff --git a/testing/framework/TestCommon.py b/testing/framework/TestCommon.py
index 1471439..34a7f6d 100644
--- a/testing/framework/TestCommon.py
+++ b/testing/framework/TestCommon.py
@@ -57,7 +57,7 @@ provided by the TestCommon class:
     test.must_not_exist('file1', ['file2', ...])
 
     test.must_not_be_empty('file')
-    
+
     test.run(options = "options to be prepended to arguments",
              stdout = "expected standard output from the program",
              stderr = "expected error output from the program",
@@ -303,7 +303,7 @@ class TestCommon(TestCmd):
         Calling test exits FAILED if search result is false
         """
         if 'b' in mode:
-            # Python 3: reading a file in binary mode returns a 
+            # Python 3: reading a file in binary mode returns a
             # bytes object. We cannot find the index of a different
             # (str) type in that, so convert.
             required = to_bytes(required)
@@ -369,6 +369,34 @@ class TestCommon(TestCmd):
             sys.stdout.write(output)
             self.fail_test()
 
+    def must_contain_single_instance_of(self, output, lines, title=None):
+        """Ensures that the specified output string (first argument)
+        contains one instance of the specified lines (second argument).
+
+        An optional third argument can be used to describe the type
+        of output being searched, and only shows up in failure output.
+
+        """
+        missing = []
+        if is_List(output):
+            output = '\n'.join(output)
+
+        counts = {}
+        for line in lines:
+            count = output.count(line)
+            if count != 1:
+                counts[line] = count
+
+        if counts:
+            if title is None:
+                title = 'output'
+            sys.stdout.write("Unexpected number of lines from %s:\n" % title)
+            for line in counts:
+                sys.stdout.write('    ' + repr(line) + ": found " + str(counts[line]) + '\n')
+            sys.stdout.write(self.banner(title + ' ') + '\n')
+            sys.stdout.write(output)
+            self.fail_test()
+
     def must_contain_any_line(self, output, lines, title=None, find=None):
         """Ensures that the specified output string (first argument)
         contains at least one of the specified lines (second argument).
@@ -581,7 +609,7 @@ class TestCommon(TestCmd):
             fsize = os.path.getsize(file)
         except OSError:
             fsize = 0
-            
+
         if fsize == 0:
             print("File is empty: `%s'" % file)
             self.fail_test(file)