fix: hold dist/ for the whole pipeline run

This commit is contained in:
Abdessamad Derraz committed 2026-10-06 12:33:59 +02:00
1 parent e071233567
commit cfb038429d
4 files changed
+77 -6

No files matched your search

+28
View File
@@ -72,6 +72,11 @@ def _strip_timestamps(text: str) -> str:
class ArtifactLockBusy(RuntimeError):
"""Raised when another process already holds the artifact directory."""
# A run that holds an artifact directory for its whole duration names it
# here for the steps it spawns: they would otherwise be refused by it.
HELD_LOCK_ENV = "RETROBIOS_HELD_ARTIFACT_DIR"
@contextlib.contextmanager
def artifact_lock(directory: str, exclusive: bool = True):
"""Serialize access to a shared artifact directory across processes.
@@ -88,6 +93,9 @@ def artifact_lock(directory: str, exclusive: bool = True):
return
os.makedirs(directory, exist_ok=True)
if os.environ.get(HELD_LOCK_ENV) == os.path.realpath(directory):
yield
return
lock_path = os.path.join(directory, ".lock")
mode = fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH
with open(lock_path, "w") as handle:
@@ -103,6 +111,26 @@ def artifact_lock(directory: str, exclusive: bool = True):
finally:
fcntl.flock(handle, fcntl.LOCK_UN)
@contextlib.contextmanager
def hold_artifact_lock(directory: str):
"""Hold a directory exclusively across several child steps.
A lock taken step by step frees the directory between them: another
run can purge the packs one step built before the next step checks
them. The children this process spawns inherit the hold.
"""
with artifact_lock(directory):
previous = os.environ.get(HELD_LOCK_ENV)
os.environ[HELD_LOCK_ENV] = os.path.realpath(directory)
try:
yield
finally:
if previous is None:
os.environ.pop(HELD_LOCK_ENV, None)
else:
os.environ[HELD_LOCK_ENV] = previous
def _build_timestamp(db: dict | None = None) -> str:
"""Timestamp for generated artifacts.
+1
View File
@@ -1733,6 +1733,7 @@ from artifacts import ( # noqa: E402,F401
write_if_changed,
ArtifactLockBusy,
artifact_lock,
hold_artifact_lock,
_TIMESTAMP_PATTERNS,
_strip_timestamps,
)
+10 -5
View File
@@ -26,6 +26,7 @@ Usage:
from __future__ import annotations
import argparse
import contextlib
import re
import subprocess
import sys
@@ -33,7 +34,7 @@ import time
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent))
from common import ArtifactLockBusy, artifact_lock
from common import ArtifactLockBusy, artifact_lock, hold_artifact_lock
def run(cmd: list[str], label: str) -> tuple[bool, str]:
@@ -340,11 +341,14 @@ def main():
# A second run on the same output directory is refused before any work:
# the database rebuild alone takes minutes, and the reader holding the
# directory would otherwise see the answer only after all of it.
if not args.skip_packs and Path(args.output_dir).is_dir():
# directory would otherwise see the answer only after all of it. The
# hold lasts until this process exits: taken step by step, it let another
# run purge the packs between the build and the integrity check, which
# then reported every platform SKIP and passed.
held = contextlib.ExitStack()
if not args.skip_packs:
try:
with artifact_lock(args.output_dir):
pass
held.enter_context(hold_artifact_lock(args.output_dir))
except ArtifactLockBusy as exc:
print(f"ERROR: {exc}")
sys.exit(1)
@@ -659,6 +663,7 @@ def main():
print(f"{'=' * 60}")
print(f" Pipeline {'COMPLETE' if all_ok else 'FINISHED WITH ERRORS'}")
print(f"{'=' * 60}")
held.close()
sys.exit(0 if all_ok else 1)
+38 -1
View File
@@ -17,7 +17,7 @@ from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
sys.path.insert(0, str(REPO_ROOT / "scripts"))
from common import ArtifactLockBusy, artifact_lock # noqa: E402
from common import ArtifactLockBusy, artifact_lock, hold_artifact_lock # noqa: E402
class ArtifactLockTest(unittest.TestCase):
@@ -156,5 +156,42 @@ class PackLockCliTest(unittest.TestCase):
self.assertIn("is in use by another run", proc.stdout)
class HeldLockTest(unittest.TestCase):
"""pipeline.py took the lock step by step: between the pack build and the
integrity check dist/ was free, another run's purge emptied it, and the
check reported every platform SKIP and passed."""
def setUp(self):
self.dir = tempfile.mkdtemp()
def _child_locks(self, env: dict) -> subprocess.CompletedProcess:
code = (
"import sys; sys.path.insert(0, 'scripts')\n"
"from common import artifact_lock\n"
f"with artifact_lock({self.dir!r}):\n pass\n"
)
return subprocess.run(
[sys.executable, "-c", code], cwd=str(REPO_ROOT), env=env,
capture_output=True, text=True, timeout=60, check=False,
)
def test_a_spawned_step_inherits_the_hold(self):
with hold_artifact_lock(self.dir):
self.assertEqual(self._child_locks(dict(os.environ)).returncode, 0)
def test_any_other_run_is_refused_for_the_whole_hold(self):
with hold_artifact_lock(self.dir):
env = {k: v for k, v in os.environ.items() if not k.startswith("RETROBIOS_")}
proc = self._child_locks(env)
self.assertNotEqual(proc.returncode, 0)
self.assertIn("ArtifactLockBusy", proc.stderr)
self.assertNotIn("RETROBIOS_HELD_ARTIFACT_DIR", os.environ)
self.assertEqual(self._child_locks(dict(os.environ)).returncode, 0)
def test_the_pipeline_holds_its_output_for_the_run(self):
source = (REPO_ROOT / "scripts" / "pipeline.py").read_text(encoding="utf-8")
self.assertIn("hold_artifact_lock(args.output_dir)", source)
if __name__ == "__main__":
unittest.main()