Skip to content

Commit

Permalink
Ensure proper cache initialization before writing
Browse files Browse the repository at this point in the history
Writing cache data is interruptible; this prevents 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.

Use json.dump instead of json.dumps + write for a tiny perf win.

Fixes pytest-dev#12167.
  • Loading branch information
tamird committed Mar 29, 2024
1 parent 12e061e commit de85150
Showing 1 changed file with 24 additions and 22 deletions.
46 changes: 24 additions & 22 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,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 +145,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,20 +182,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")
except OSError as exc:
Expand All @@ -201,19 +198,24 @@ def set(self, key: str, value: object) -> None:
)
else:
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")

cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG")
cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT)
json.dump(value, f, ensure_ascii=False, indent=2)

def _ensure_cache_dir_and_supporting_files(self) -> None:
"""Create the cache dir and its supporting files."""
self._cachedir.mkdir(exist_ok=True, parents=True)

for file, content in (
("README.md", README_CONTENT),
(".gitignore", "# Created by pytest automatically.\n*\n"),
("CACHEDIR.TAG", CACHEDIR_TAG_CONTENT),
):
path = self._cachedir.joinpath(file)
if not path.exists():
match content:
case str():
path.write_text(content)
case bytes():
path.write_bytes(content)


class LFPluginCollWrapper:
Expand Down

0 comments on commit de85150

Please sign in to comment.