Skip to content

Commit 4f938d3

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 381593c commit 4f938d3

File tree

2 files changed

+35
-18
lines changed

2 files changed

+35
-18
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

+34-18
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
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
@@ -123,6 +125,10 @@ def warn(self, fmt: str, *, _ispytest: bool = False, **args: object) -> None:
123125
stacklevel=3,
124126
)
125127

128+
def _mkdir(self, path: Path) -> None:
129+
self._ensure_cache_dir_and_supporting_files()
130+
path.mkdir(exist_ok=True, parents=True)
131+
126132
def mkdir(self, name: str) -> Path:
127133
"""Return a directory path object with the given name.
128134
@@ -141,7 +147,7 @@ def mkdir(self, name: str) -> Path:
141147
if len(path.parts) > 1:
142148
raise ValueError("name is not allowed to contain path separators")
143149
res = self._cachedir.joinpath(self._CACHE_PREFIX_DIRS, path)
144-
res.mkdir(exist_ok=True, parents=True)
150+
self._mkdir(res)
145151
return res
146152

147153
def _getvaluepath(self, key: str) -> Path:
@@ -178,19 +184,13 @@ def set(self, key: str, value: object) -> None:
178184
"""
179185
path = self._getvaluepath(key)
180186
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)
187+
self._mkdir(path.parent)
186188
except OSError as exc:
187189
self.warn(
188190
f"could not create cache path {path}: {exc}",
189191
_ispytest=True,
190192
)
191193
return
192-
if not cache_dir_exists_already:
193-
self._ensure_supporting_files()
194194
data = json.dumps(value, ensure_ascii=False, indent=2)
195195
try:
196196
f = path.open("w", encoding="UTF-8")
@@ -203,17 +203,33 @@ def set(self, key: str, value: object) -> None:
203203
with f:
204204
f.write(data)
205205

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")
210-
211-
gitignore_path = self._cachedir.joinpath(".gitignore")
212-
msg = "# Created by pytest automatically.\n*\n"
213-
gitignore_path.write_text(msg, encoding="UTF-8")
206+
def _ensure_cache_dir_and_supporting_files(self) -> None:
207+
"""Create the cache dir and its supporting files."""
208+
if self._cachedir.is_dir():
209+
return
214210

215-
cachedir_tag_path = self._cachedir.joinpath("CACHEDIR.TAG")
216-
cachedir_tag_path.write_bytes(CACHEDIR_TAG_CONTENT)
211+
with tempfile.TemporaryDirectory() as d:
212+
path = Path(d)
213+
with open(path.joinpath("README.md"), "xt", encoding="UTF-8") as f:
214+
f.write(README_CONTENT)
215+
with open(path.joinpath(".gitignore"), "xt", encoding="UTF-8") as f:
216+
f.write("# Created by pytest automatically.\n*\n")
217+
with open(path.joinpath("CACHEDIR.TAG"), "xb") as f:
218+
f.write(CACHEDIR_TAG_CONTENT)
219+
220+
self._cachedir.parent.mkdir(parents=True, exist_ok=True)
221+
try:
222+
path.rename(self._cachedir)
223+
except OSError:
224+
shutil.copytree(d, self._cachedir)
225+
else:
226+
# Create a directory in place of the one we just moved so that
227+
# `TemporaryDirectory`'s cleanup doesn't complain.
228+
#
229+
# TODO: pass ignore_cleanup_errors=True when we no longer support python < 3.10. See
230+
# https://github.com/python/cpython/issues/74168. Note that passing delete=False
231+
# would do the wrong thing in case of errors and isn't supported until python 3.12.
232+
path.mkdir()
217233

218234

219235
class LFPluginCollWrapper:

0 commit comments

Comments
 (0)