Skip to content

Commit c24b40b

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 4489528 commit c24b40b

File tree

2 files changed

+38
-17
lines changed

2 files changed

+38
-17
lines changed

changelog/12167.trivial.rst

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
cache: create supporting files (``CACHEDIR.TAG``, ``.gitignore``, etc.) in the cache directory before writing cache data, reducing the probability of those files being lost.

src/_pytest/cacheprovider.py

+37-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,35 @@ 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+
shutil.move(d, self._cachedir)
231+
# Create a directory in place of the one we just moved so that `TemporaryDirectory`'s
232+
# cleanup doesn't complain.
233+
#
234+
# TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10. See
235+
# https://github.com/python/cpython/issues/74168.
236+
os.mkdir(d)
217237

218238

219239
class LFPluginCollWrapper:

0 commit comments

Comments
 (0)