Add a tag to the cachedir

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

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

SCons/CacheDir.py

@@ -29,13 +29,20 @@
 import os
 import stat
 import sys
+import tempfile
 import uuid
 
 import SCons.Action
 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 @@ def CachePushFunc(target, source, env):
 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 @@ def __init__(self, path) -> None:
         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 @@ def is_enabled(self) -> bool:
     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)

SCons/CacheDirTests.py

@@ -31,6 +31,7 @@
 from TestCmd import TestCmd, IS_WINDOWS, IS_ROOT
 
 import SCons.CacheDir
+import SCons.Node.FS
 
 built_it = None
 
@@ -62,15 +63,11 @@ def get_CacheDir(self):
         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,20 +80,19 @@ def File(self, name, bsig=None, action=Action()):
         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"""
 
         # Verify how the cachepath() method determines the name
         # 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 @@ def my_collect(list, hash_format=None):
         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 @@ def test_throws_correct_on_OSError(self) -> None:
         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 @@ def __setitem__(self, name, value) -> None:
 
         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 @@ def test_raise_environment_error_on_invalid_json(self) -> None:
         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 @@ def test_CacheRetrieveSilent(self) -> None:
 
     def test_CachePush(self) -> None:
         """Test the CachePush() function"""
-
         save_CachePush = SCons.CacheDir.CachePush
-
         SCons.CacheDir.CachePush = self.push
 
         try:
@@ -309,7 +319,6 @@ def test_CachePush(self) -> None:
 
     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 @@ def mkdir(dir, mode: int=0) -> None:
 
     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')