Skip to content

Commit

Permalink
Initialize cache directory in isolation
Browse files Browse the repository at this point in the history
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 pytest-dev#12167.
  • Loading branch information
tamird committed Apr 2, 2024
1 parent 4489528 commit 6b1db13
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 17 deletions.
1 change: 1 addition & 0 deletions changelog/12167.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cache: create cache directory supporting files (``CACHEDIR.TAG``, ``.gitignore``, etc.) in a temporary directory to provide atomic semantics.
60 changes: 43 additions & 17 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,24 @@
import json
import os
from pathlib import Path
import shutil
import tempfile
from typing import Dict
from typing import final
from typing import Generator
from typing import Iterable
from typing import List
from typing import Optional
from typing import Set
from typing import Tuple
from typing import Union

from .pathlib import resolve_from_str
from .pathlib import rm_rf
from .reports import CollectReport
from _pytest import nodes
from _pytest._io import TerminalWriter
from _pytest.compat import assert_never
from _pytest.config import Config
from _pytest.config import ExitCode
from _pytest.config import hookimpl
Expand Down Expand Up @@ -123,6 +127,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.
Expand All @@ -141,7 +149,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:
Expand Down Expand Up @@ -178,19 +186,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")
Expand All @@ -203,17 +205,41 @@ 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")
def _ensure_cache_dir_and_supporting_files(self) -> None:
"""Create the cache dir and its supporting files."""
if self._cachedir.is_dir():
return

gitignore_path = self._cachedir.joinpath(".gitignore")
msg = "# Created by pytest automatically.\n*\n"
gitignore_path.write_text(msg, encoding="UTF-8")
files: Iterable[Tuple[str, Union[str, bytes]]] = (
("README.md", README_CONTENT),
(".gitignore", "# Created by pytest automatically.\n*\n"),
("CACHEDIR.TAG", CACHEDIR_TAG_CONTENT),
)

cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG")
cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT)
with tempfile.TemporaryDirectory() as d:
for file, content in files:
file = os.path.join(d, file)
if isinstance(content, str):
with open(file, "xt", encoding="UTF-8") as f:
f.write(content)
elif isinstance(content, bytes):
with open(file, "xb") as f:
f.write(content)
else:
assert_never(content)

self._cachedir.parent.mkdir(parents=True, exist_ok=True)
try:
os.rename(d, self._cachedir)
# 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.
os.mkdir(d)
except OSError:
shutil.copytree(d, self._cachedir)


class LFPluginCollWrapper:
Expand Down

0 comments on commit 6b1db13

Please sign in to comment.