From 4f938d3b9890e5da7cc4b6ad39f0a0222cb3f078 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 29 Mar 2024 15:51:51 +0000 Subject: [PATCH] Initialize cache directory in isolation Creating and initializing the cache directory is interruptible; this avoids a pathological case where interrupting a cache write can cause the cache directory to never be properly initialized with its supporting files. Unify `Cache.mkdir` with `Cache.set` while I'm here so the former also properly initializes the cache directory. Closes #12167. --- changelog/12167.trivial.rst | 1 + src/_pytest/cacheprovider.py | 52 +++++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 18 deletions(-) create mode 100644 changelog/12167.trivial.rst diff --git a/changelog/12167.trivial.rst b/changelog/12167.trivial.rst new file mode 100644 index 00000000000..da9363420e6 --- /dev/null +++ b/changelog/12167.trivial.rst @@ -0,0 +1 @@ +cache: create cache directory supporting files (``CACHEDIR.TAG``, ``.gitignore``, etc.) in a temporary directory to provide atomic semantics. diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 81703ddac44..3ab1b7cdf5c 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -7,6 +7,8 @@ import json import os from pathlib import Path +import shutil +import tempfile from typing import Dict from typing import final from typing import Generator @@ -123,6 +125,10 @@ def warn(self, fmt: str, *, _ispytest: bool = False, **args: object) -> None: stacklevel=3, ) + def _mkdir(self, path: Path) -> None: + self._ensure_cache_dir_and_supporting_files() + path.mkdir(exist_ok=True, parents=True) + def mkdir(self, name: str) -> Path: """Return a directory path object with the given name. @@ -141,7 +147,7 @@ def mkdir(self, name: str) -> Path: if len(path.parts) > 1: raise ValueError("name is not allowed to contain path separators") res = self._cachedir.joinpath(self._CACHE_PREFIX_DIRS, path) - res.mkdir(exist_ok=True, parents=True) + self._mkdir(res) return res def _getvaluepath(self, key: str) -> Path: @@ -178,19 +184,13 @@ def set(self, key: str, value: object) -> None: """ path = self._getvaluepath(key) try: - if path.parent.is_dir(): - cache_dir_exists_already = True - else: - cache_dir_exists_already = self._cachedir.exists() - path.parent.mkdir(exist_ok=True, parents=True) + self._mkdir(path.parent) except OSError as exc: self.warn( f"could not create cache path {path}: {exc}", _ispytest=True, ) return - if not cache_dir_exists_already: - self._ensure_supporting_files() data = json.dumps(value, ensure_ascii=False, indent=2) try: f = path.open("w", encoding="UTF-8") @@ -203,17 +203,33 @@ def set(self, key: str, value: object) -> None: with f: f.write(data) - def _ensure_supporting_files(self) -> None: - """Create supporting files in the cache dir that are not really part of the cache.""" - readme_path = self._cachedir / "README.md" - readme_path.write_text(README_CONTENT, encoding="UTF-8") - - gitignore_path = self._cachedir.joinpath(".gitignore") - msg = "# Created by pytest automatically.\n*\n" - gitignore_path.write_text(msg, encoding="UTF-8") + def _ensure_cache_dir_and_supporting_files(self) -> None: + """Create the cache dir and its supporting files.""" + if self._cachedir.is_dir(): + return - cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG") - cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT) + with tempfile.TemporaryDirectory() as d: + path = Path(d) + with open(path.joinpath("README.md"), "xt", encoding="UTF-8") as f: + f.write(README_CONTENT) + with open(path.joinpath(".gitignore"), "xt", encoding="UTF-8") as f: + f.write("# Created by pytest automatically.\n*\n") + with open(path.joinpath("CACHEDIR.TAG"), "xb") as f: + f.write(CACHEDIR_TAG_CONTENT) + + self._cachedir.parent.mkdir(parents=True, exist_ok=True) + try: + path.rename(self._cachedir) + except OSError: + shutil.copytree(d, self._cachedir) + else: + # Create a directory in place of the one we just moved so that + # `TemporaryDirectory`'s cleanup doesn't complain. + # + # TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10. See + # https://github.com/python/cpython/issues/74168. Note that passing delete=False + # would do the wrong thing in case of errors and isn't supported until python 3.12. + path.mkdir() class LFPluginCollWrapper: