Skip to content

Commit bf91b27

Browse files
committed
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.
1 parent 1125296 commit bf91b27

File tree

2 files changed

+44
-17
lines changed

2 files changed

+44
-17
lines changed

changelog/12167.trivial.rst

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
cache: create cache directory supporting files (``CACHEDIR.TAG``, ``.gitignore``, etc.) in a temporary directory to provide atomic semantics.

src/_pytest/cacheprovider.py

+43-17
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,24 @@
77
import json
88
import os
99
from pathlib import Path
10+
import shutil
11+
import tempfile
1012
from typing import Dict
1113
from typing import final
1214
from typing import Generator
1315
from typing import Iterable
1416
from typing import List
1517
from typing import Optional
1618
from typing import Set
19+
from typing import Tuple
1720
from typing import Union
1821

1922
from .pathlib import resolve_from_str
2023
from .pathlib import rm_rf
2124
from .reports import CollectReport
2225
from _pytest import nodes
2326
from _pytest._io import TerminalWriter
27+
from _pytest.compat import assert_never
2428
from _pytest.config import Config
2529
from _pytest.config import ExitCode
2630
from _pytest.config import hookimpl
@@ -123,6 +127,10 @@ def warn(self, fmt: str, *, _ispytest: bool = False, **args: object) -> None:
123127
stacklevel=3,
124128
)
125129

130+
def _mkdir(self, path: Path) -> None:
131+
self._ensure_cache_dir_and_supporting_files()
132+
path.mkdir(exist_ok=True, parents=True)
133+
126134
def mkdir(self, name: str) -> Path:
127135
"""Return a directory path object with the given name.
128136
@@ -141,7 +149,7 @@ def mkdir(self, name: str) -> Path:
141149
if len(path.parts) > 1:
142150
raise ValueError("name is not allowed to contain path separators")
143151
res = self._cachedir.joinpath(self._CACHE_PREFIX_DIRS, path)
144-
res.mkdir(exist_ok=True, parents=True)
152+
self._mkdir(res)
145153
return res
146154

147155
def _getvaluepath(self, key: str) -> Path:
@@ -178,19 +186,13 @@ def set(self, key: str, value: object) -> None:
178186
"""
179187
path = self._getvaluepath(key)
180188
try:
181-
if path.parent.is_dir():
182-
cache_dir_exists_already = True
183-
else:
184-
cache_dir_exists_already = self._cachedir.exists()
185-
path.parent.mkdir(exist_ok=True, parents=True)
189+
self._mkdir(path.parent)
186190
except OSError as exc:
187191
self.warn(
188192
f"could not create cache path {path}: {exc}",
189193
_ispytest=True,
190194
)
191195
return
192-
if not cache_dir_exists_already:
193-
self._ensure_supporting_files()
194196
data = json.dumps(value, ensure_ascii=False, indent=2)
195197
try:
196198
f = path.open("w", encoding="UTF-8")
@@ -203,17 +205,41 @@ def set(self, key: str, value: object) -> None:
203205
with f:
204206
f.write(data)
205207

206-
def _ensure_supporting_files(self) -> None:
207-
"""Create supporting files in the cache dir that are not really part of the cache."""
208-
readme_path = self._cachedir / "README.md"
209-
readme_path.write_text(README_CONTENT, encoding="UTF-8")
208+
def _ensure_cache_dir_and_supporting_files(self) -> None:
209+
"""Create the cache dir and its supporting files."""
210+
if self._cachedir.is_dir():
211+
return
210212

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

215-
cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG")
216-
cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT)
219+
with tempfile.TemporaryDirectory() as d:
220+
for file, content in files:
221+
file = os.path.join(d, file)
222+
if isinstance(content, str):
223+
with open(file, "xt", encoding="UTF-8") as f:
224+
f.write(content)
225+
elif isinstance(content, bytes):
226+
with open(file, "xb") as f:
227+
f.write(content)
228+
else:
229+
assert_never(content)
230+
231+
self._cachedir.parent.mkdir(parents=True, exist_ok=True)
232+
try:
233+
os.rename(d, self._cachedir)
234+
# Create a directory in place of the one we just moved so that
235+
# `TemporaryDirectory`'s cleanup doesn't complain.
236+
#
237+
# TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10. See
238+
# https://github.com/python/cpython/issues/74168. Note that passing delete=False
239+
# would do the wrong thing in case of errors and isn't supported until python 3.12.
240+
os.mkdir(d)
241+
except OSError:
242+
shutil.copytree(d, self._cachedir)
217243

218244

219245
class LFPluginCollWrapper:

0 commit comments

Comments
 (0)