Add a tag to the cachedir

Since there are now two files to make when a cachedir is created,
use the temporary-dir -> rename technique.

CacheDir tests no longer pre-create the cache directory, they should
be verifying it's created properly upon request (one unit test still
makes sure passing an existing empty directory works, too).

Signed-off-by: Mats Wichmann <mats@linux.com>

21 files changed, 169 insertions, 101 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 4161b1b..c53fce3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -15,7 +15,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
   From Joseph Brill:
     - Added error handling when creating MS VC detection debug log file specified by
       SCONS_MSCOMMON_DEBUG
-      
+
   From Alex James:
     - On Darwin, PermissionErrors are now handled while trying to access
       /etc/paths.d. This may occur if SCons is invoked in a sandboxed
@@ -71,6 +71,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Skip running a few validation tests if the user is root and the test is
       not designed to work for the root user.
     - Clarify documentation of Repository() in manpage and user guide.
+    - Add a tag to each CacheDir to let systems ignore backing it up
+      (per https://bford.info/cachedir/). Update the way a CacheDir
+      is created, since it now has to create two files.
 
 
 RELEASE 4.8.1 -  Tue, 03 Sep 2024 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 9f60ef2..baa9d1d 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -33,8 +33,13 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   keyword arguments to Builder calls (or manually, through the
   undocumented Override method), were modified not to "leak" on item deletion.
   The item will now not be deleted from the base environment.
+
 - Added support for tracking beamer themes in the LaTeX scanner.
 
+- Add a tag to each CacheDir to let systems ignore backing it up
+  (per https://bford.info/cachedir/). Update the way a CacheDir
+  is created, since it now has to create two files.
+
 FIXES
 -----
 - PackageVariable now does what the documentation always said it does
diff --git a/SCons/CacheDir.py b/SCons/CacheDir.py
index 0174793..7f8deb5 100644
--- a/SCons/CacheDir.py
+++ b/SCons/CacheDir.py
@@ -29,6 +29,7 @@ import json
 import os
 import stat
 import sys
+import tempfile
 import uuid
 
 import SCons.Action
@@ -36,6 +37,12 @@ import SCons.Errors
 import SCons.Warnings
 import SCons.Util
 
+CACHE_PREFIX_LEN = 2  # first two characters used as subdirectory name
+CACHE_TAG = (
+    b"Signature: 8a477f597d28d172789f06886806bc55\n"
+    b"# SCons cache directory - see https://bford.info/cachedir/\n"
+)
+
 cache_enabled = True
 cache_debug = False
 cache_force = False
@@ -67,20 +74,20 @@ def CacheRetrieveFunc(target, source, env) -> int:
         fs.chmod(t.get_internal_path(), stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
     return 0
 
-def CacheRetrieveString(target, source, env) -> None:
+def CacheRetrieveString(target, source, env) -> str:
     t = target[0]
     fs = t.fs
     cd = env.get_CacheDir()
     cachedir, cachefile = cd.cachepath(t)
     if t.fs.exists(cachefile):
         return "Retrieved `%s' from cache" % t.get_internal_path()
-    return None
+    return ""
 
 CacheRetrieve = SCons.Action.Action(CacheRetrieveFunc, CacheRetrieveString)
 
 CacheRetrieveSilent = SCons.Action.Action(CacheRetrieveFunc, None)
 
-def CachePushFunc(target, source, env):
+def CachePushFunc(target, source, env) -> None:
     if cache_readonly:
         return
 
@@ -134,8 +141,7 @@ CachePush = SCons.Action.Action(CachePushFunc, None)
 class CacheDir:
 
     def __init__(self, path) -> None:
-        """
-        Initialize a CacheDir object.
+        """Initialize a CacheDir object.
 
         The cache configuration is stored in the object. It
         is read from the config file in the supplied path if
@@ -147,53 +153,97 @@ class CacheDir:
         self.path = path
         self.current_cache_debug = None
         self.debugFP = None
-        self.config = dict()
-        if path is None:
-            return
-
-        self._readconfig(path)
+        self.config = {}
+        if path is not None:
+            self._readconfig(path)
+
+    def _add_config(self, path: str) -> None:
+        """Create the cache config file in *path*.
+
+        Locking isn't necessary in the normal case - when the cachedir is
+        being created - because it's written to a unique directory first,
+        before the directory is renamed. But it is legal to call CacheDir
+        with an existing directory, which may be missing the config file,
+        and in that case we do need locking. Simpler to always lock.
+        """
+        config_file = os.path.join(path, 'config')
+        # TODO: this breaks the "unserializable config object" test which
+        #   does some crazy stuff, so for now don't use setdefault. It does
+        #   seem like it would be better to preserve an exisiting value.
+        # self.config.setdefault('prefix_len', CACHE_PREFIX_LEN)
+        self.config['prefix_len'] = CACHE_PREFIX_LEN
+        with SCons.Util.FileLock(config_file, timeout=5, writer=True), open(
+            config_file, "x"
+        ) as config:
+            try:
+                json.dump(self.config, config)
+            except Exception:
+                msg = "Failed to write cache configuration for " + path
+                raise SCons.Errors.SConsEnvironmentError(msg)
 
+        # Add the tag file "carelessly" - the contents are not used by SCons
+        # so we don't care about the chance of concurrent writes.
+        try:
+            tagfile = os.path.join(path, "CACHEDIR.TAG")
+            with open(tagfile, 'xb') as cachedir_tag:
+                cachedir_tag.write(CACHE_TAG)
+        except FileExistsError:
+            pass
 
-    def _readconfig(self, path):
-        """
-        Read the cache config.
+    def _mkdir_atomic(self, path: str) -> bool:
+        """Create cache directory at *path*.
 
-        If directory or config file do not exist, create.  Take advantage
-        of Py3 capability in os.makedirs() and in file open(): just try
-        the operation and handle failure appropriately.
+        Uses directory renaming to avoid races.  If we are actually
+        creating the dir, populate it with the metadata files at the
+        same time as that's the safest way. But it's not illegal to point
+        CacheDir at an existing directory that wasn't a cache previously,
+        so we may have to do that elsewhere, too.
 
-        Omit the check for old cache format, assume that's old enough
-        there will be none of those left to worry about.
+        Returns:
+            ``True`` if it we created the dir, ``False`` if already existed,
 
-        :param path: path to the cache directory
+        Raises:
+            SConsEnvironmentError: if we tried and failed to create the cache.
         """
-        config_file = os.path.join(path, 'config')
+        directory = os.path.abspath(path)
+        if os.path.exists(directory):
+            return False
+
         try:
-            # still use a try block even with exist_ok, might have other fails
-            os.makedirs(path, exist_ok=True)
-        except OSError:
+            tempdir = tempfile.TemporaryDirectory(dir=os.path.dirname(directory))
+        except OSError as e:
             msg = "Failed to create cache directory " + path
-            raise SCons.Errors.SConsEnvironmentError(msg)
+            raise SCons.Errors.SConsEnvironmentError(msg) from e
+        self._add_config(tempdir.name)
+        with tempdir:
+            try:
+                os.rename(tempdir.name, directory)
+                return True
+            except Exception as e:
+                # did someone else get there first?
+                if os.path.isdir(directory):
+                    return False
+                msg = "Failed to create cache directory " + path
+                raise SCons.Errors.SConsEnvironmentError(msg) from e
+
+    def _readconfig(self, path: str) -> None:
+        """Read the cache config from *path*.
 
+        If directory or config file do not exist, create and populate.
+        """
+        config_file = os.path.join(path, 'config')
+        created = self._mkdir_atomic(path)
+        if not created and not os.path.isfile(config_file):
+            # Could have been passed an empty directory
+            self._add_config(path)
         try:
-            with SCons.Util.FileLock(config_file, timeout=5, writer=True), open(
-                config_file, "x"
+            with SCons.Util.FileLock(config_file, timeout=5, writer=False), open(
+                config_file
             ) as config:
-                self.config['prefix_len'] = 2
-                try:
-                    json.dump(self.config, config)
-                except Exception:
-                    msg = "Failed to write cache configuration for " + path
-                    raise SCons.Errors.SConsEnvironmentError(msg)
-        except FileExistsError:
-            try:
-                with SCons.Util.FileLock(config_file, timeout=5, writer=False), open(
-                    config_file
-                ) as config:
-                    self.config = json.load(config)
-            except (ValueError, json.decoder.JSONDecodeError):
-                msg = "Failed to read cache configuration for " + path
-                raise SCons.Errors.SConsEnvironmentError(msg)
+                self.config = json.load(config)
+        except (ValueError, json.decoder.JSONDecodeError):
+            msg = "Failed to read cache configuration for " + path
+            raise SCons.Errors.SConsEnvironmentError(msg)
 
     def CacheDebug(self, fmt, target, cachefile) -> None:
         if cache_debug != self.current_cache_debug:
@@ -252,7 +302,7 @@ class CacheDir:
     def is_readonly(self) -> bool:
         return cache_readonly
 
-    def get_cachedir_csig(self, node):
+    def get_cachedir_csig(self, node) -> str:
         cachedir, cachefile = self.cachepath(node)
         if cachefile and os.path.exists(cachefile):
             return SCons.Util.hash_file_signature(cachefile, SCons.Node.FS.File.hash_chunksize)
diff --git a/SCons/CacheDirTests.py b/SCons/CacheDirTests.py
index 3fbab4e..0ecd502 100644
--- a/SCons/CacheDirTests.py
+++ b/SCons/CacheDirTests.py
@@ -31,6 +31,7 @@ import stat
 from TestCmd import TestCmd, IS_WINDOWS, IS_ROOT
 
 import SCons.CacheDir
+import SCons.Node.FS
 
 built_it = None
 
@@ -62,15 +63,11 @@ class Environment:
         return self.cachedir
 
 class BaseTestCase(unittest.TestCase):
-    """
-    Base fixtures common to our other unittest classes.
-    """
+    """Base fixtures common to our other unittest classes."""
+
     def setUp(self) -> None:
         self.test = TestCmd(workdir='')
-
-        import SCons.Node.FS
         self.fs = SCons.Node.FS.FS()
-
         self._CacheDir = SCons.CacheDir.CacheDir('cache')
 
     def File(self, name, bsig=None, action=Action()):
@@ -83,13 +80,11 @@ class BaseTestCase(unittest.TestCase):
         return node
 
     def tearDown(self) -> None:
-        os.remove(os.path.join(self._CacheDir.path, 'config'))
-        os.rmdir(self._CacheDir.path)
+        shutil.rmtree(self._CacheDir.path)
 
 class CacheDirTestCase(BaseTestCase):
-    """
-    Test calling CacheDir code directly.
-    """
+    """Test calling CacheDir code directly."""
+
     def test_cachepath(self) -> None:
         """Test the cachepath() method"""
 
@@ -97,6 +92,7 @@ class CacheDirTestCase(BaseTestCase):
         # of the file in cache.
         def my_collect(list, hash_format=None):
             return list[0]
+
         save_collect = SCons.Util.hash_collect
         SCons.Util.hash_collect = my_collect
 
@@ -111,6 +107,21 @@ class CacheDirTestCase(BaseTestCase):
         finally:
             SCons.Util.hash_collect = save_collect
 
+class CacheDirExistsTestCase(unittest.TestCase):
+    """Test passing an existing but not setup cache directory."""
+
+    def setUp(self) -> None:
+        self.test = TestCmd(workdir='')
+        self.test.subdir('ex-cache')  # force an empty dir
+        cache = self.test.workpath('ex-cache')
+        self.fs = SCons.Node.FS.FS()
+        self._CacheDir = SCons.CacheDir.CacheDir(cache)
+
+    def test_existing_cachedir(self) -> None:
+        """Test the setup happened even though cache already existed."""
+        assert os.path.exists(self.test.workpath('ex-cache', 'config'))
+        assert os.path.exists(self.test.workpath('ex-cache', 'CACHEDIR.TAG'))
+
 class ExceptionTestCase(unittest.TestCase):
     """Test that the correct exceptions are thrown by CacheDir."""
 
@@ -147,13 +158,14 @@ class ExceptionTestCase(unittest.TestCase):
         test.writable(privileged_dir, True)
         shutil.rmtree(privileged_dir)
 
-
     def test_throws_correct_when_failed_to_write_configfile(self) -> None:
+        """Test for correct error if cache config file cannot be created."""
+
         class Unserializable:
-            """A class which the JSON should not be able to serialize"""
+            """A class which the JSON module should not be able to serialize."""
 
             def __init__(self, oldconfig) -> None:
-                self.something = 1 # Make the object unserializable
+                self.something = 1  # Make the object unserializable
                 # Pretend to be the old config just enough
                 self.__dict__["prefix_len"] = oldconfig["prefix_len"]
 
@@ -168,16 +180,17 @@ class ExceptionTestCase(unittest.TestCase):
 
         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:
+        with self.assertRaises(SCons.Errors.SConsEnvironmentError) as cm:
             self._CacheDir._readconfig(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)
+        self.assertEqual(
+            str(cm.exception),
+            "Failed to write cache configuration for " + self._CacheDir.path,
+        )
 
     def test_raise_environment_error_on_invalid_json(self) -> None:
         config_file = os.path.join(self._CacheDir.path, "config")
@@ -188,17 +201,16 @@ class ExceptionTestCase(unittest.TestCase):
         with open(config_file, "w") as cfg:
             cfg.write(content)
 
-        try:
-            # Construct a new cache dir that will try to read the invalid config
+        with self.assertRaises(SCons.Errors.SConsEnvironmentError) as cm:
+            # Construct a new cachedir 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)
+        self.assertEqual(
+            str(cm.exception),
+            "Failed to read cache configuration for " + self._CacheDir.path,
+        )
 
 class FileTestCase(BaseTestCase):
-    """
-    Test calling CacheDir code through Node.FS.File interfaces.
-    """
+    """Test calling CacheDir code through Node.FS.File interfaces."""
     # These tests were originally in Nodes/FSTests.py and got moved
     # when the CacheDir support was refactored into its own module.
     # Look in the history for Node/FSTests.py if any of this needs
@@ -274,9 +286,7 @@ class FileTestCase(BaseTestCase):
 
     def test_CachePush(self) -> None:
         """Test the CachePush() function"""
-
         save_CachePush = SCons.CacheDir.CachePush
-
         SCons.CacheDir.CachePush = self.push
 
         try:
@@ -309,7 +319,6 @@ class FileTestCase(BaseTestCase):
 
     def test_warning(self) -> None:
         """Test raising a warning if we can't copy a file to cache."""
-
         test = TestCmd(workdir='')
 
         save_copy2 = shutil.copy2
@@ -337,7 +346,6 @@ class FileTestCase(BaseTestCase):
 
     def test_no_strfunction(self) -> None:
         """Test handling no strfunction() for an action."""
-
         save_CacheRetrieveSilent = SCons.CacheDir.CacheRetrieveSilent
 
         f8 = self.File("cd.f8", 'f8_bsig')
diff --git a/test/CacheDir/CacheDir.py b/test/CacheDir/CacheDir.py
index bd8d674..05134e6 100644
--- a/test/CacheDir/CacheDir.py
+++ b/test/CacheDir/CacheDir.py
@@ -41,7 +41,7 @@ src_ccc_out = test.workpath('src', 'ccc.out')
 src_cat_out = test.workpath('src', 'cat.out')
 src_all = test.workpath('src', 'all')
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """\
 DefaultEnvironment(tools=[])
@@ -84,8 +84,12 @@ test.must_not_exist(src_bbb_out)
 test.must_not_exist(src_ccc_out)
 test.must_not_exist(src_all)
 # Even if you do -n, the cache will be configured.
-test.fail_test(os.listdir(cache) != ['config'])
-
+expect = ['CACHEDIR.TAG', 'config']
+found = sorted(os.listdir(cache))
+test.fail_test(
+    expect != found,
+    message=f"expected cachedir contents {expect}, found {found}",
+)
 # Verify that a normal build works correctly, and clean up.
 # This should populate the cache with our derived files.
 test.run(chdir = 'src', arguments = '.')
diff --git a/test/CacheDir/CacheDir_TryCompile.py b/test/CacheDir/CacheDir_TryCompile.py
index b00c5d4..c9ab126 100644
--- a/test/CacheDir/CacheDir_TryCompile.py
+++ b/test/CacheDir/CacheDir_TryCompile.py
@@ -38,7 +38,7 @@ test = TestSCons.TestSCons()
 
 cache = test.workpath('cache')
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """\
 DefaultEnvironment(tools=[])
diff --git a/test/CacheDir/NoCache.py b/test/CacheDir/NoCache.py
index e1cecee..611b555 100644
--- a/test/CacheDir/NoCache.py
+++ b/test/CacheDir/NoCache.py
@@ -31,7 +31,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'alpha', 'beta')
+test.subdir('alpha', 'beta')
 
 sconstruct = """
 DefaultEnvironment(tools=[])
diff --git a/test/CacheDir/SideEffect.py b/test/CacheDir/SideEffect.py
index 61c9bbc..ee808d6 100644
--- a/test/CacheDir/SideEffect.py
+++ b/test/CacheDir/SideEffect.py
@@ -31,7 +31,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'work')
+test.subdir('work')
 
 cache = test.workpath('cache')
 
diff --git a/test/CacheDir/VariantDir.py b/test/CacheDir/VariantDir.py
index 9fc82c9..8c44917 100644
--- a/test/CacheDir/VariantDir.py
+++ b/test/CacheDir/VariantDir.py
@@ -33,7 +33,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 cache = test.workpath('cache')
 cat_out = test.workpath('cat.out')
diff --git a/test/CacheDir/debug.py b/test/CacheDir/debug.py
index 0ba6bbb..a8c4e83 100644
--- a/test/CacheDir/debug.py
+++ b/test/CacheDir/debug.py
@@ -36,7 +36,7 @@ import TestSCons
 
 test = TestSCons.TestSCons(match=TestSCons.match_re)
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 cache = test.workpath('cache')
 debug_out = test.workpath('cache-debug.out')
diff --git a/test/CacheDir/environment.py b/test/CacheDir/environment.py
index 17a4f26..23e3508 100644
--- a/test/CacheDir/environment.py
+++ b/test/CacheDir/environment.py
@@ -42,7 +42,7 @@ src_ccc_out = test.workpath('src', 'ccc.out')
 src_cat_out = test.workpath('src', 'cat.out')
 src_all = test.workpath('src', 'all')
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """\
 DefaultEnvironment(tools=[])
@@ -87,7 +87,12 @@ test.must_not_exist(src_bbb_out)
 test.must_not_exist(src_ccc_out)
 test.must_not_exist(src_all)
 # Even if you do -n, the cache will be configured.
-test.fail_test(os.listdir(cache) != ['config'])
+expect = ['CACHEDIR.TAG', 'config']
+found = sorted(os.listdir(cache))
+test.fail_test(
+    expect != found,
+    message=f"expected cachedir contents {expect}, found {found}",
+)
 
 # Verify that a normal build works correctly, and clean up.
 # This should populate the cache with our derived files.
diff --git a/test/CacheDir/multi-targets.py b/test/CacheDir/multi-targets.py
index 72c7e66..7c606af 100644
--- a/test/CacheDir/multi-targets.py
+++ b/test/CacheDir/multi-targets.py
@@ -31,7 +31,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'multiple')
+test.subdir('multiple')
 
 cache = test.workpath('cache')
 
diff --git a/test/CacheDir/multiple-targets.py b/test/CacheDir/multiple-targets.py
index 1218e9b..dfde453 100644
--- a/test/CacheDir/multiple-targets.py
+++ b/test/CacheDir/multiple-targets.py
@@ -32,8 +32,6 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache')
-
 test.write('SConstruct', """\
 DefaultEnvironment(tools=[])
 def touch(env, source, target):
diff --git a/test/CacheDir/option--cd.py b/test/CacheDir/option--cd.py
index 692207d..df9ab47 100644
--- a/test/CacheDir/option--cd.py
+++ b/test/CacheDir/option--cd.py
@@ -33,7 +33,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """
 DefaultEnvironment(tools=[])
diff --git a/test/CacheDir/option--cf.py b/test/CacheDir/option--cf.py
index 4750b40..b34d706 100644
--- a/test/CacheDir/option--cf.py
+++ b/test/CacheDir/option--cf.py
@@ -34,7 +34,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """
 DefaultEnvironment(tools=[])
@@ -88,7 +88,6 @@ test.up_to_date(chdir = 'src', arguments = '.')
 # Blow away and recreate the CacheDir, then verify that --cache-force
 # repopulates the cache with the local built targets.  DO NOT CLEAN UP.
 shutil.rmtree(test.workpath('cache'))
-test.subdir('cache')
 
 test.run(chdir = 'src', arguments = '--cache-force .')
 
@@ -106,7 +105,6 @@ test.fail_test(os.path.exists(test.workpath('src', 'cat.out')))
 # Blow away and recreate the CacheDir, then verify that --cache-populate
 # repopulates the cache with the local built targets.  DO NOT CLEAN UP.
 shutil.rmtree(test.workpath('cache'))
-test.subdir('cache')
 
 test.run(chdir = 'src', arguments = '--cache-populate .')
 
diff --git a/test/CacheDir/option--cr.py b/test/CacheDir/option--cr.py
index 6ff6974..4c423cf 100644
--- a/test/CacheDir/option--cr.py
+++ b/test/CacheDir/option--cr.py
@@ -33,7 +33,7 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'src')
+test.subdir('src')
 
 test.write(['src', 'SConstruct'], """
 DefaultEnvironment(tools=[])
diff --git a/test/CacheDir/option--cs.py b/test/CacheDir/option--cs.py
index ce48ac0..62c0026 100644
--- a/test/CacheDir/option--cs.py
+++ b/test/CacheDir/option--cs.py
@@ -38,7 +38,7 @@ _obj = TestSCons._obj
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'src1', 'src2')
+test.subdir('src1', 'src2')
 
 test.write(['src1', 'build.py'], r"""
 import sys
@@ -63,7 +63,7 @@ def cat(env, source, target):
                 f.write(f2.read())
 
 DefaultEnvironment(tools=[])  # test speedup
-env = Environment(tools=[], 
+env = Environment(tools=[],
                   BUILDERS={'Internal':Builder(action=cat),
                             'External':Builder(action=r'%(_python_)s build.py $TARGET $SOURCES')})
 env.External('aaa.out', 'aaa.in')
diff --git a/test/CacheDir/scanner-target.py b/test/CacheDir/scanner-target.py
index dd8791d..4aa3692 100644
--- a/test/CacheDir/scanner-target.py
+++ b/test/CacheDir/scanner-target.py
@@ -35,8 +35,6 @@ import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache')
-
 test.write('SConstruct', """\
 DefaultEnvironment(tools=[])
 import SCons
diff --git a/test/CacheDir/source-scanner.py b/test/CacheDir/source-scanner.py
index 0dda4f4..2ff19ce 100644
--- a/test/CacheDir/source-scanner.py
+++ b/test/CacheDir/source-scanner.py
@@ -39,7 +39,7 @@ test = TestSCons.TestSCons()
 
 cache = test.workpath('cache')
 
-test.subdir('cache', 'subdir')
+test.subdir('subdir')
 
 test.write(['subdir', 'SConstruct'], """\
 DefaultEnvironment(tools=[])
diff --git a/test/CacheDir/up-to-date-q.py b/test/CacheDir/up-to-date-q.py
index c8fa1e3..5c71ab9 100644
--- a/test/CacheDir/up-to-date-q.py
+++ b/test/CacheDir/up-to-date-q.py
@@ -39,19 +39,19 @@ Thanks to dvitek for the test case.
 # 1. Set up two identical C project directories called 'alpha' and
 #    'beta', which use the same cache
 # 2. Invoke scons on 'alpha'
-# 3. Invoke scons on 'beta', which successfully draws output 
+# 3. Invoke scons on 'beta', which successfully draws output
 #    files from the cache
 # 4. Invoke scons again, asserting (with -q) that 'beta' is up to date
 #
 # Step 4 failed in 0.96.93.  In practice, this problem would lead to
-# lots of unecessary fetches from the cache during incremental 
+# lots of unecessary fetches from the cache during incremental
 # builds (because they behaved like non-incremental builds).
 
 import TestSCons
 
 test = TestSCons.TestSCons()
 
-test.subdir('cache', 'alpha', 'beta')
+test.subdir('alpha', 'beta')
 
 foo_c = """
 int main(void){ return 0; }
diff --git a/test/CacheDir/value_dependencies.py b/test/CacheDir/value_dependencies.py
index b34401d..37c6153 100644
--- a/test/CacheDir/value_dependencies.py
+++ b/test/CacheDir/value_dependencies.py
@@ -37,7 +37,6 @@ import TestSCons
 test = TestSCons.TestSCons()
 
 test.dir_fixture('value_dependencies')
-test.subdir('cache')
 
 # First build, populates the cache
 test.run(arguments='.')