fix: remove potential partial cache-population in case of error (#2892)
fix: remove potential partial extraction in case of error Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot
parent
cdb170b8c6
commit
6cd2d197c1
@@ -30,7 +30,13 @@ from cibuildwheel.frontend import (
|
||||
from cibuildwheel.logger import log
|
||||
from cibuildwheel.util import resources
|
||||
from cibuildwheel.util.cmd import call, shell
|
||||
from cibuildwheel.util.file import CIBW_CACHE_PATH, copy_test_sources, download, move_file
|
||||
from cibuildwheel.util.file import (
|
||||
CIBW_CACHE_PATH,
|
||||
copy_test_sources,
|
||||
download,
|
||||
move_file,
|
||||
remove_on_error,
|
||||
)
|
||||
from cibuildwheel.util.helpers import prepare_command
|
||||
from cibuildwheel.util.packaging import find_compatible_wheel
|
||||
from cibuildwheel.util.python_build_standalone import create_python_build_standalone_environment
|
||||
@@ -182,7 +188,8 @@ def setup_target_python(config: PythonConfiguration, build_path: Path) -> Path:
|
||||
python_tgz = CIBW_CACHE_PATH / config.url.rpartition("/")[-1]
|
||||
with FileLock(f"{python_tgz}.lock"):
|
||||
if not python_tgz.exists():
|
||||
download(config.url, python_tgz, sha256=config.sha256)
|
||||
with remove_on_error(python_tgz):
|
||||
download(config.url, python_tgz, sha256=config.sha256)
|
||||
|
||||
python_dir = build_path / "python"
|
||||
python_dir.mkdir()
|
||||
|
||||
@@ -25,7 +25,13 @@ from cibuildwheel.logger import log
|
||||
from cibuildwheel.platforms.macos import install_cpython as install_build_cpython
|
||||
from cibuildwheel.util import resources
|
||||
from cibuildwheel.util.cmd import call, shell, split_command
|
||||
from cibuildwheel.util.file import CIBW_CACHE_PATH, copy_test_sources, download, move_file
|
||||
from cibuildwheel.util.file import (
|
||||
CIBW_CACHE_PATH,
|
||||
copy_test_sources,
|
||||
download,
|
||||
move_file,
|
||||
remove_on_error,
|
||||
)
|
||||
from cibuildwheel.util.helpers import prepare_command, unwrap_preserving_paragraphs
|
||||
from cibuildwheel.util.packaging import find_compatible_wheel
|
||||
from cibuildwheel.venv import constraint_flags, virtualenv
|
||||
@@ -140,8 +146,9 @@ def install_target_cpython(tmp: Path, config: PythonConfiguration, free_threadin
|
||||
if not installation_path.exists():
|
||||
downloaded_tar_gz = tmp / ios_python_tar_gz
|
||||
download(config.url, downloaded_tar_gz, sha256=config.sha256)
|
||||
installation_path.mkdir(parents=True, exist_ok=True)
|
||||
call("tar", "-C", installation_path, "-xf", downloaded_tar_gz)
|
||||
with remove_on_error(installation_path):
|
||||
installation_path.mkdir(parents=True)
|
||||
call("tar", "-C", installation_path, "-xf", downloaded_tar_gz)
|
||||
downloaded_tar_gz.unlink()
|
||||
|
||||
return installation_path
|
||||
|
||||
@@ -27,7 +27,13 @@ from cibuildwheel.frontend import (
|
||||
from cibuildwheel.logger import log
|
||||
from cibuildwheel.util import resources
|
||||
from cibuildwheel.util.cmd import call, shell
|
||||
from cibuildwheel.util.file import CIBW_CACHE_PATH, copy_test_sources, download, move_file
|
||||
from cibuildwheel.util.file import (
|
||||
CIBW_CACHE_PATH,
|
||||
copy_test_sources,
|
||||
download,
|
||||
move_file,
|
||||
remove_on_error,
|
||||
)
|
||||
from cibuildwheel.util.helpers import prepare_command, unwrap
|
||||
from cibuildwheel.util.packaging import find_compatible_wheel, get_pip_version
|
||||
from cibuildwheel.venv import constraint_flags, find_uv, target_marker_env, virtualenv
|
||||
@@ -196,7 +202,8 @@ def install_pypy(tmp: Path, url: str, sha256: str) -> Path:
|
||||
downloaded_tar_bz2 = tmp / pypy_tar_bz2
|
||||
download(url, downloaded_tar_bz2, sha256=sha256)
|
||||
installation_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
call("tar", "-C", installation_path.parent, "-xf", downloaded_tar_bz2)
|
||||
with remove_on_error(installation_path):
|
||||
call("tar", "-C", installation_path.parent, "-xf", downloaded_tar_bz2)
|
||||
downloaded_tar_bz2.unlink()
|
||||
return installation_path / "bin" / "pypy3"
|
||||
|
||||
@@ -210,9 +217,17 @@ def install_graalpy(tmp: Path, url: str, sha256: str) -> Path:
|
||||
if not installation_path.exists():
|
||||
downloaded_archive = tmp / graalpy_archive
|
||||
download(url, downloaded_archive, sha256=sha256)
|
||||
installation_path.mkdir(parents=True)
|
||||
# GraalPy top-folder name is inconsistent with archive name
|
||||
call("tar", "-C", installation_path, "--strip-components=1", "-xzf", downloaded_archive)
|
||||
with remove_on_error(installation_path):
|
||||
installation_path.mkdir(parents=True)
|
||||
# GraalPy top-folder name is inconsistent with archive name
|
||||
call(
|
||||
"tar",
|
||||
"-C",
|
||||
installation_path,
|
||||
"--strip-components=1",
|
||||
"-xzf",
|
||||
downloaded_archive,
|
||||
)
|
||||
downloaded_archive.unlink()
|
||||
return installation_path / "bin" / "graalpy"
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ from cibuildwheel.util.file import (
|
||||
extract_tar,
|
||||
extract_zip,
|
||||
move_file,
|
||||
remove_on_error,
|
||||
)
|
||||
from cibuildwheel.util.helpers import prepare_command, unwrap, unwrap_preserving_paragraphs
|
||||
from cibuildwheel.util.packaging import find_compatible_wheel, get_pip_version
|
||||
@@ -92,10 +93,11 @@ def ensure_node(major_version: str) -> Path:
|
||||
with TemporaryDirectory() as tmp_path:
|
||||
archive = Path(tmp_path) / f"{name}.{ext}"
|
||||
download(url, archive)
|
||||
if ext == "zip":
|
||||
extract_zip(archive, path.parent)
|
||||
else:
|
||||
extract_tar(archive, path.parent)
|
||||
with remove_on_error(path):
|
||||
if ext == "zip":
|
||||
extract_zip(archive, path.parent)
|
||||
else:
|
||||
extract_tar(archive, path.parent)
|
||||
assert path.exists()
|
||||
if not IS_WIN:
|
||||
return path / "bin"
|
||||
@@ -110,18 +112,19 @@ def install_emscripten(env: dict[str, str], version: str, xbuildenv_cache_path:
|
||||
with FileLock(CIBW_CACHE_PATH / "emscripten.lock"):
|
||||
if emscripten_dir.exists():
|
||||
return emscripten_dir
|
||||
call(
|
||||
"pyodide",
|
||||
"xbuildenv",
|
||||
"install-emscripten",
|
||||
"--force",
|
||||
"--version",
|
||||
version,
|
||||
"--path",
|
||||
str(xbuildenv_cache_path),
|
||||
env=env,
|
||||
cwd=CIBW_CACHE_PATH,
|
||||
)
|
||||
with remove_on_error(emscripten_dir):
|
||||
call(
|
||||
"pyodide",
|
||||
"xbuildenv",
|
||||
"install-emscripten",
|
||||
"--force",
|
||||
"--version",
|
||||
version,
|
||||
"--path",
|
||||
str(xbuildenv_cache_path),
|
||||
env=env,
|
||||
cwd=CIBW_CACHE_PATH,
|
||||
)
|
||||
assert emscripten_dir.exists()
|
||||
return emscripten_dir
|
||||
|
||||
@@ -208,17 +211,18 @@ def install_xbuildenv(env: dict[str, str], xbuildenv_cache_path: Path, pyodide_v
|
||||
env.pop("PYODIDE_ROOT", None)
|
||||
|
||||
# Install the xbuildenv
|
||||
call(
|
||||
"pyodide",
|
||||
"xbuildenv",
|
||||
"install",
|
||||
"--path",
|
||||
str(xbuildenv_cache_path),
|
||||
pyodide_version,
|
||||
env=env,
|
||||
cwd=CIBW_CACHE_PATH,
|
||||
)
|
||||
assert pyodide_root.exists()
|
||||
with remove_on_error(xbuildenv_cache_path / pyodide_version):
|
||||
call(
|
||||
"pyodide",
|
||||
"xbuildenv",
|
||||
"install",
|
||||
"--path",
|
||||
str(xbuildenv_cache_path),
|
||||
pyodide_version,
|
||||
env=env,
|
||||
cwd=CIBW_CACHE_PATH,
|
||||
)
|
||||
assert pyodide_root.exists()
|
||||
|
||||
return str(pyodide_root)
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ from cibuildwheel.util.file import (
|
||||
download,
|
||||
extract_zip,
|
||||
move_file,
|
||||
remove_on_error,
|
||||
)
|
||||
from cibuildwheel.util.helpers import prepare_command, unwrap
|
||||
from cibuildwheel.util.packaging import find_compatible_wheel, get_pip_version
|
||||
@@ -112,7 +113,8 @@ def _ensure_nuget() -> Path:
|
||||
nuget = CIBW_CACHE_PATH / "nuget.exe"
|
||||
with FileLock(str(nuget) + ".lock"):
|
||||
if not nuget.exists():
|
||||
download("https://dist.nuget.org/win-x86-commandline/latest/nuget.exe", nuget)
|
||||
with remove_on_error(nuget):
|
||||
download("https://dist.nuget.org/win-x86-commandline/latest/nuget.exe", nuget)
|
||||
return nuget
|
||||
|
||||
|
||||
@@ -128,7 +130,8 @@ def install_cpython(configuration: PythonConfiguration, arch: str | None = None)
|
||||
with FileLock(str(base_output_dir) + f"-{version}{free_threaded_str}-{arch}.lock"):
|
||||
if not installation_path.exists():
|
||||
nuget = _ensure_nuget()
|
||||
call(nuget, "install", *nuget_args)
|
||||
with remove_on_error(installation_path.parent):
|
||||
call(nuget, "install", *nuget_args)
|
||||
return installation_path / "python.exe"
|
||||
|
||||
|
||||
@@ -144,8 +147,9 @@ def install_pypy(tmp: Path, arch: str, url: str, sha256: str) -> Path:
|
||||
if not installation_path.exists():
|
||||
pypy_zip = tmp / zip_filename
|
||||
download(url, pypy_zip, sha256=sha256)
|
||||
# Extract to the parent directory because the zip file still contains a directory
|
||||
extract_zip(pypy_zip, installation_path.parent)
|
||||
with remove_on_error(installation_path):
|
||||
# Extract to the parent directory because the zip file still contains a directory
|
||||
extract_zip(pypy_zip, installation_path.parent)
|
||||
return installation_path / "python.exe"
|
||||
|
||||
|
||||
@@ -158,8 +162,9 @@ def install_graalpy(tmp: Path, url: str, sha256: str) -> Path:
|
||||
if not installation_path.exists():
|
||||
graalpy_zip = tmp / zip_filename
|
||||
download(url, graalpy_zip, sha256=sha256)
|
||||
# Extract to the parent directory because the zip file still contains a directory
|
||||
extract_zip(graalpy_zip, installation_path.parent)
|
||||
with remove_on_error(installation_path):
|
||||
# Extract to the parent directory because the zip file still contains a directory
|
||||
extract_zip(graalpy_zip, installation_path.parent)
|
||||
return installation_path / "bin" / "graalpy.exe"
|
||||
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import ssl
|
||||
import tarfile
|
||||
import time
|
||||
import urllib.request
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path, PurePath
|
||||
from typing import Final
|
||||
from zipfile import ZipFile
|
||||
@@ -18,7 +19,7 @@ from cibuildwheel.errors import FatalError
|
||||
|
||||
TYPE_CHECKING = False
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
from collections.abc import Callable, Generator
|
||||
|
||||
DEFAULT_CIBW_CACHE_PATH: Final[Path] = user_cache_path(appname="cibuildwheel", appauthor="pypa")
|
||||
CIBW_CACHE_PATH: Final[Path] = Path(
|
||||
@@ -26,6 +27,22 @@ CIBW_CACHE_PATH: Final[Path] = Path(
|
||||
).resolve()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def remove_on_error(path: Path) -> Generator[None, None, None]:
|
||||
try:
|
||||
yield
|
||||
except BaseException as original_exception:
|
||||
try:
|
||||
if path.is_dir() and not path.is_symlink():
|
||||
shutil.rmtree(path)
|
||||
elif path.exists() or path.is_symlink():
|
||||
path.unlink()
|
||||
except BaseException as cleanup_exception:
|
||||
msg = f"Failed to remove {path}. Please remove it manually."
|
||||
raise BaseExceptionGroup(msg, [original_exception, cleanup_exception]) from None
|
||||
raise
|
||||
|
||||
|
||||
def ensure_cache_sentinel(cache_path: Path) -> None:
|
||||
"""Create a sentinel file to mark a cibuildwheel cache directory.
|
||||
|
||||
|
||||
@@ -10,7 +10,7 @@ import typing
|
||||
|
||||
from filelock import FileLock
|
||||
|
||||
from cibuildwheel.util.file import download, extract_tar
|
||||
from cibuildwheel.util.file import download, extract_tar, remove_on_error
|
||||
from cibuildwheel.util.resources import PYTHON_BUILD_STANDALONE_RELEASES
|
||||
|
||||
TYPE_CHECKING = False
|
||||
@@ -157,7 +157,8 @@ def _download_or_get_from_cache(
|
||||
return asset_cache_path
|
||||
|
||||
print(f"Downloading python_build_standalone: {asset_url} to {asset_cache_path}")
|
||||
download(asset_url, asset_cache_path, sha256=sha256 or None)
|
||||
with remove_on_error(asset_cache_path):
|
||||
download(asset_url, asset_cache_path, sha256=sha256 or None)
|
||||
return asset_cache_path
|
||||
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ from packaging.version import Version
|
||||
|
||||
from cibuildwheel.util import resources
|
||||
from cibuildwheel.util.cmd import call
|
||||
from cibuildwheel.util.file import CIBW_CACHE_PATH, download
|
||||
from cibuildwheel.util.file import CIBW_CACHE_PATH, download, remove_on_error
|
||||
|
||||
TYPE_CHECKING = False
|
||||
if TYPE_CHECKING:
|
||||
@@ -57,7 +57,8 @@ def _ensure_virtualenv(version: str) -> tuple[Path, Version]:
|
||||
path = CIBW_CACHE_PATH / f"virtualenv-{version}.pyz"
|
||||
with FileLock(str(path) + ".lock"):
|
||||
if not path.exists():
|
||||
download(url, path, sha256=sha256)
|
||||
with remove_on_error(path):
|
||||
download(url, path, sha256=sha256)
|
||||
return (path, Version(version))
|
||||
|
||||
|
||||
|
||||
+54
-1
@@ -1,3 +1,4 @@
|
||||
import re
|
||||
import textwrap
|
||||
from pathlib import Path, PurePath
|
||||
from unittest.mock import Mock, call
|
||||
@@ -6,7 +7,7 @@ import pytest
|
||||
|
||||
from cibuildwheel import errors
|
||||
from cibuildwheel.ci import fix_ansi_codes_for_github_actions
|
||||
from cibuildwheel.util.file import copy_test_sources
|
||||
from cibuildwheel.util.file import copy_test_sources, remove_on_error
|
||||
from cibuildwheel.util.helpers import (
|
||||
FlexibleVersion,
|
||||
format_safe,
|
||||
@@ -395,6 +396,58 @@ def test_copy_test_sources_alternate_copy_into(sample_project: Path) -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_remove_on_error_keeps_paths(tmp_path: Path) -> None:
|
||||
file_path = tmp_path / "file.txt"
|
||||
file_path.write_text("data")
|
||||
dir_path = tmp_path / "dir"
|
||||
dir_path.mkdir()
|
||||
|
||||
for path in (file_path, dir_path):
|
||||
with remove_on_error(path):
|
||||
pass
|
||||
|
||||
assert file_path.exists()
|
||||
assert dir_path.exists()
|
||||
|
||||
|
||||
def test_remove_on_error_file_or_tree(tmp_path: Path) -> None:
|
||||
file_path = tmp_path / "file.txt"
|
||||
file_path.write_text("data")
|
||||
dir_path = tmp_path / "dir"
|
||||
nested_file_path = dir_path / "nested.txt"
|
||||
nested_file_path.parent.mkdir()
|
||||
nested_file_path.write_text("data")
|
||||
|
||||
for path in (file_path, dir_path):
|
||||
with pytest.raises(RuntimeError), remove_on_error(path):
|
||||
raise RuntimeError
|
||||
|
||||
assert not file_path.exists()
|
||||
assert not dir_path.exists()
|
||||
|
||||
|
||||
def test_remove_on_error_cleanup_failure(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
path = tmp_path / "cleanup-failure"
|
||||
path.mkdir()
|
||||
original_exception = RuntimeError("original error")
|
||||
cleanup_exception = PermissionError("cleanup error")
|
||||
|
||||
def _raise_cleanup_error(_: Path) -> None:
|
||||
raise cleanup_exception
|
||||
|
||||
monkeypatch.setattr("cibuildwheel.util.file.shutil.rmtree", _raise_cleanup_error)
|
||||
with (
|
||||
pytest.raises(
|
||||
BaseExceptionGroup,
|
||||
match=re.escape(f"Failed to remove {path}. Please remove it manually."),
|
||||
) as caught_exception,
|
||||
remove_on_error(path),
|
||||
):
|
||||
raise original_exception
|
||||
|
||||
assert caught_exception.value.exceptions == (original_exception, cleanup_exception)
|
||||
|
||||
|
||||
def test_unwrap() -> None:
|
||||
assert (
|
||||
unwrap("""
|
||||
|
||||
Reference in New Issue
Block a user