fix(preflight): asset mode retries a move that fails, names the cause and rolls back (#197)
A part of the stack that will not move from the unpack folder into place is retried for about 30 seconds; each retry removes what reached the target and unpacks the entry afresh from the archive, so the entry ends up there once and whole. If it still fails, both scripts take back everything they moved and exit 1 with the system's message and "nothing was installed". New test hook CHEMENU_PREFLIGHT_MOVE_FAIL=<entry>:<n>[:partial]. Files changed: - CHANGES.md - INSTALL.md - VERSION - instructions/preflight.md - tools/CONTRACT.md - tools/chemenu/tests/conftest.py - tools/chemenu/tests/test_preflight.py - tools/chemenu/tests/test_preflight_pwsh.py - tools/preflight.ps1 - tools/preflight.sh Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SnAJ7Z3CpVD3PRbN73QtU2
This commit is contained in:
1 parent
5915b07383
commit
5c9be38aa8
10 files changed
+379
-47
No files matched your search
@@ -37,6 +37,7 @@ _WIKITOOL_ENV = (
|
||||
"CHEMENU_PREFLIGHT_LONGPATHS",
|
||||
"CHEMENU_PREFLIGHT_POLICY",
|
||||
"CHEMENU_PREFLIGHT_MARKED",
|
||||
"CHEMENU_PREFLIGHT_MOVE_FAIL",
|
||||
) + tuple(var for var, _harness in HARNESS_ENV_VARS)
|
||||
|
||||
# Environment git reads for identity or for where its repo lives. A stray
|
||||
|
||||
@@ -19,6 +19,7 @@ import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import tarfile
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -607,6 +608,100 @@ def test_set_and_the_exit_code_pass_through_to_the_tree_copy(asset, shell):
|
||||
assert second.recorded()["tools"]["rg"] == str(elsewhere)
|
||||
|
||||
|
||||
# --- an entry that will not move up (Gitea #197) -------------------------------------
|
||||
#
|
||||
# CHEMENU_PREFLIGHT_MOVE_FAIL stands in for the file a virus scanner holds open for a
|
||||
# moment; with :partial each failed attempt has already moved one file of the entry up,
|
||||
# which is what a move that falls back to copying and deleting leaves behind.
|
||||
|
||||
ARCHIVED_TOOLS = sorted((*STACK_FILES, "requirements.txt"))
|
||||
MOVE_ATTEMPTS = 8
|
||||
|
||||
|
||||
def tools_as_unpacked(root: Path) -> list[str]:
|
||||
"""Every path below <root>/tools, the tree copy's own .venv left out."""
|
||||
tools = root / "tools"
|
||||
return sorted(path.relative_to(tools).as_posix() for path in tools.rglob("*")
|
||||
if path.relative_to(tools).parts[0] != ".venv")
|
||||
|
||||
|
||||
def folder_state(folder: Path) -> dict[str, bytes | None]:
|
||||
"""What a folder holds, file contents included - to compare before against after."""
|
||||
return {path.relative_to(folder).as_posix(): (path.read_bytes() if path.is_file() else None)
|
||||
for path in sorted(folder.rglob("*"))}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("shell", SHELLS)
|
||||
@pytest.mark.parametrize("partial", ["", ":partial"])
|
||||
def test_a_move_that_fails_twice_is_retried_into_a_whole_entry(asset, shell, partial):
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = f"tools:2{partial}"
|
||||
result = asset.run("--archive", str(asset.tarball), shell=shell)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
assert "Preflight passed" in result.stdout
|
||||
assert tools_as_unpacked(asset.root) == ARCHIVED_TOOLS
|
||||
assert not asset.leftovers()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("shell", SHELLS)
|
||||
@pytest.mark.parametrize("partial", ["", ":partial"])
|
||||
@pytest.mark.parametrize("into", [False, True])
|
||||
def test_a_move_that_never_works_is_taken_back(asset, shell, partial, into):
|
||||
target = asset.base / "wiki" if into else asset.root
|
||||
(target / ".git").mkdir(parents=True)
|
||||
(target / ".git" / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8")
|
||||
before = folder_state(target)
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = f"tools:always{partial}"
|
||||
args = ("--archive", str(asset.tarball), *(("--into", str(target)) if into else ()))
|
||||
|
||||
started = time.monotonic()
|
||||
result = asset.run(*args, shell=shell)
|
||||
assert result.returncode == 1, result.stdout + result.stderr
|
||||
# Eight attempts, and none of the 30 seconds of pauses between them under the hook.
|
||||
assert time.monotonic() - started < 20
|
||||
assert "could not move tools into" in result.stderr
|
||||
assert f"Permission denied (CHEMENU_PREFLIGHT_MOVE_FAIL, attempt {MOVE_ATTEMPTS})" in result.stderr
|
||||
assert "nothing was installed" in result.stderr
|
||||
assert folder_state(target) == before
|
||||
assert not asset.leftovers()
|
||||
|
||||
del asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"]
|
||||
again = asset.run(*args, shell=shell)
|
||||
assert again.returncode == 0, again.stdout + again.stderr
|
||||
assert tools_as_unpacked(target) == ARCHIVED_TOOLS
|
||||
|
||||
|
||||
def add_to_release(asset: AssetMachine, files: dict[str, str]) -> None:
|
||||
"""Rebuild the asset's release with <files> (paths below the top folder) added."""
|
||||
stage = asset.base / "restage"
|
||||
with tarfile.open(asset.tarball, "r:gz") as archive:
|
||||
archive.extractall(stage, filter="data")
|
||||
top = stage / f"chemenu-stack-{RELEASE_VERSION}"
|
||||
for name, text in files.items():
|
||||
(top / name).parent.mkdir(parents=True, exist_ok=True)
|
||||
(top / name).write_text(text, encoding="utf-8")
|
||||
with tarfile.open(asset.tarball, "w:gz") as archive:
|
||||
archive.add(top, arcname=top.name)
|
||||
digest = hashlib.sha256(asset.tarball.read_bytes()).hexdigest()
|
||||
(asset.tarball.parent / f"{asset.tarball.name}.sha256").write_text(
|
||||
f"{digest} {asset.tarball.name}\n", encoding="utf-8")
|
||||
|
||||
|
||||
# Moved before tools/ by both scripts: the glob sorts them first, Get-ChildItem lists
|
||||
# folders first and kb/ sorts before tools/.
|
||||
MOVED_FIRST = {"AGENTS.md": "# agents\n", "kb/index.md": "# index\n"}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("shell", SHELLS)
|
||||
def test_an_entry_that_moved_before_the_failure_is_taken_back_too(asset, shell):
|
||||
add_to_release(asset, MOVED_FIRST)
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = "tools:always"
|
||||
result = asset.run("--archive", str(asset.tarball), shell=shell)
|
||||
assert result.returncode == 1, result.stdout + result.stderr
|
||||
assert "nothing was installed" in result.stderr
|
||||
assert sorted(path.name for path in asset.root.iterdir()) == [asset.script.name]
|
||||
assert not asset.leftovers()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("shell", SHELLS)
|
||||
@pytest.mark.parametrize("flag", ["--into", "--archive"])
|
||||
def test_asset_options_are_a_usage_error_inside_a_tree(machine, shell, flag):
|
||||
|
||||
@@ -20,14 +20,15 @@ import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from chemenu import prerequisites
|
||||
from chemenu.tests.test_preflight import (
|
||||
DOWNLOAD_BASE, ECHO_PYTHON, SHELLS, TOOLS, AssetMachine, Machine, _machine_with_root_of,
|
||||
assert_guidance,
|
||||
ARCHIVED_TOOLS, DOWNLOAD_BASE, ECHO_PYTHON, MOVE_ATTEMPTS, MOVED_FIRST, SHELLS, TOOLS, AssetMachine,
|
||||
Machine, _machine_with_root_of, add_to_release, assert_guidance, folder_state, tools_as_unpacked,
|
||||
)
|
||||
|
||||
PWSH = shutil.which("pwsh")
|
||||
@@ -311,6 +312,53 @@ def test_set_and_the_exit_code_pass_through_to_the_tree_copy(asset):
|
||||
assert second.recorded()["tools"]["rg"] == str(elsewhere)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("partial", ["", ":partial"])
|
||||
def test_a_move_that_fails_twice_is_retried_into_a_whole_entry(asset, partial):
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = f"tools:2{partial}"
|
||||
result = _asset_run(asset, "--archive", str(asset.tarball))
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
assert "Preflight passed" in result.stdout
|
||||
assert tools_as_unpacked(asset.root) == ARCHIVED_TOOLS
|
||||
assert not asset.leftovers()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("partial", ["", ":partial"])
|
||||
@pytest.mark.parametrize("into", [False, True])
|
||||
def test_a_move_that_never_works_is_taken_back(asset, partial, into):
|
||||
target = asset.base / "wiki" if into else asset.root
|
||||
(target / ".git").mkdir(parents=True)
|
||||
(target / ".git" / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8")
|
||||
before = folder_state(target)
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = f"tools:always{partial}"
|
||||
args = ("--archive", str(asset.tarball), *(("--into", str(target)) if into else ()))
|
||||
|
||||
started = time.monotonic()
|
||||
result = _asset_run(asset, *args)
|
||||
assert result.returncode == 1, result.stdout + result.stderr
|
||||
# Eight attempts, and none of the 30 seconds of pauses between them under the hook.
|
||||
assert time.monotonic() - started < 20
|
||||
assert "could not move tools into" in result.stderr
|
||||
assert f"is denied. (CHEMENU_PREFLIGHT_MOVE_FAIL, attempt {MOVE_ATTEMPTS})" in result.stderr
|
||||
assert "nothing was installed" in result.stderr
|
||||
assert folder_state(target) == before
|
||||
assert not asset.leftovers()
|
||||
|
||||
del asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"]
|
||||
again = _asset_run(asset, *args)
|
||||
assert again.returncode == 0, again.stdout + again.stderr
|
||||
assert tools_as_unpacked(target) == ARCHIVED_TOOLS
|
||||
|
||||
|
||||
def test_an_entry_that_moved_before_the_failure_is_taken_back_too(asset):
|
||||
add_to_release(asset, MOVED_FIRST)
|
||||
asset.extra_env["CHEMENU_PREFLIGHT_MOVE_FAIL"] = "tools:always"
|
||||
result = _asset_run(asset, "--archive", str(asset.tarball))
|
||||
assert result.returncode == 1, result.stdout + result.stderr
|
||||
assert "nothing was installed" in result.stderr
|
||||
assert sorted(path.name for path in asset.root.iterdir()) == [asset.script.name]
|
||||
assert not asset.leftovers()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("flag", ["--into", "--archive"])
|
||||
def test_asset_options_are_a_usage_error_inside_a_tree(machine, flag):
|
||||
result = _preflight(machine, flag, str(machine.base))
|
||||
|
||||
Reference in new issue
Block a user