Skip to content

Commit 7192d31

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 7192d31

File tree

3 files changed

+35
-20
lines changed

3 files changed

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

+32-18
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import json
88
import os
99
from pathlib import Path
10+
import tempfile
1011
from typing import Dict
1112
from typing import final
1213
from typing import Generator
@@ -123,6 +124,10 @@ def warn(self, fmt: str, *, _ispytest: bool = False, **args: object) -> None:
123124
stacklevel=3,
124125
)
125126

127+
def _mkdir(self, path: Path) -> None:
128+
self._ensure_cache_dir_and_supporting_files()
129+
path.mkdir(exist_ok=True, parents=True)
130+
126131
def mkdir(self, name: str) -> Path:
127132
"""Return a directory path object with the given name.
128133
@@ -141,7 +146,7 @@ def mkdir(self, name: str) -> Path:
141146
if len(path.parts) > 1:
142147
raise ValueError("name is not allowed to contain path separators")
143148
res = self._cachedir.joinpath(self._CACHE_PREFIX_DIRS, path)
144-
res.mkdir(exist_ok=True, parents=True)
149+
self._mkdir(res)
145150
return res
146151

147152
def _getvaluepath(self, key: str) -> Path:
@@ -178,19 +183,13 @@ def set(self, key: str, value: object) -> None:
178183
"""
179184
path = self._getvaluepath(key)
180185
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)
186+
self._mkdir(path.parent)
186187
except OSError as exc:
187188
self.warn(
188189
f"could not create cache path {path}: {exc}",
189190
_ispytest=True,
190191
)
191192
return
192-
if not cache_dir_exists_already:
193-
self._ensure_supporting_files()
194193
data = json.dumps(value, ensure_ascii=False, indent=2)
195194
try:
196195
f = path.open("w", encoding="UTF-8")
@@ -203,17 +202,32 @@ def set(self, key: str, value: object) -> None:
203202
with f:
204203
f.write(data)
205204

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")
205+
def _ensure_cache_dir_and_supporting_files(self) -> None:
206+
"""Create the cache dir and its supporting files."""
207+
if self._cachedir.is_dir():
208+
return
214209

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

218232

219233
class LFPluginCollWrapper:

testing/test_assertrewrite.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -1731,8 +1731,8 @@ def test_cwd_changed(self, pytester: Pytester, monkeypatch) -> None:
17311731
import os
17321732
import tempfile
17331733
1734-
with tempfile.TemporaryDirectory() as d:
1735-
os.chdir(d)
1734+
with tempfile.TemporaryDirectory() as newpath:
1735+
os.chdir(newpath)
17361736
""",
17371737
"test_test.py": """\
17381738
def test():

0 commit comments

Comments
 (0)