mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-23 20:29:06 -05:00
fix: hold the pipeline's fixed outputs for a run
The pipeline locked only --output-dir while it also writes database.json, install/, README.md and docs/. A run on a scratch output directory could start beside the run on dist/ and rewrite them. Every run now holds the repository's .lock first.
This commit is contained in:
1 parent
43337c6719
commit
12cc71b634
5 files changed
+75
-8
No files matched your search
@@ -217,6 +217,33 @@ def hold_artifact_lock(directory: str) -> Iterator[None]:
|
||||
else:
|
||||
os.environ[HELD_LOCK_ENV] = previous
|
||||
|
||||
|
||||
# What a pipeline run writes at fixed paths, whatever its --output-dir.
|
||||
REPOSITORY_OUTPUTS = ("database.json", "install/", "README.md", "CONTRIBUTING.md", "docs/")
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def hold_repository_outputs(root: str = ".") -> Iterator[None]:
|
||||
"""Hold the outputs a pipeline writes at fixed paths, for the whole run.
|
||||
|
||||
The output-directory lock let a run on tmp/packtest start beside a run
|
||||
on dist/: both rewrote database.json, and the second to reach the
|
||||
manifest step found install/ held after minutes of work, or, when the
|
||||
steps did not overlap, rewrote the committed manifests from its own
|
||||
state. The lock file is the repository root's .lock.
|
||||
"""
|
||||
stack = contextlib.ExitStack()
|
||||
try:
|
||||
stack.enter_context(artifact_lock(root))
|
||||
except ArtifactLockBusy as exc:
|
||||
raise ArtifactLockBusy(
|
||||
f"another pipeline run is writing {', '.join(REPOSITORY_OUTPUTS)} "
|
||||
f"(lock: {os.path.join(root, '.lock')}). Wait for it to finish."
|
||||
) from exc
|
||||
with stack:
|
||||
yield
|
||||
|
||||
|
||||
def _build_timestamp(db: dict | None = None) -> str:
|
||||
"""Timestamp for generated artifacts.
|
||||
|
||||
|
||||
@@ -2810,6 +2810,7 @@ from artifacts import ( # noqa: E402,F401
|
||||
ArtifactLockBusy,
|
||||
artifact_lock,
|
||||
hold_artifact_lock,
|
||||
hold_repository_outputs,
|
||||
_TIMESTAMP_PATTERNS,
|
||||
_strip_timestamps,
|
||||
)
|
||||
|
||||
+13
-5
@@ -68,6 +68,7 @@ from common import (
|
||||
artifact_lock,
|
||||
github_token,
|
||||
hold_artifact_lock,
|
||||
hold_repository_outputs,
|
||||
load_data_dir_registry,
|
||||
)
|
||||
from refresh_data_dirs import hold_data_dirs
|
||||
@@ -700,13 +701,20 @@ def main() -> None:
|
||||
# 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.
|
||||
# database.json, install/, README.md and docs/ sit at fixed paths, so
|
||||
# every run holds them too: a run on a scratch --output-dir otherwise
|
||||
# rewrote them beside the run on dist/.
|
||||
held = contextlib.ExitStack()
|
||||
if not args.skip_packs:
|
||||
try:
|
||||
try:
|
||||
held.enter_context(
|
||||
hold_repository_outputs(str(Path(__file__).resolve().parent.parent))
|
||||
)
|
||||
if not args.skip_packs:
|
||||
held.enter_context(hold_artifact_lock(args.output_dir))
|
||||
except ArtifactLockBusy as exc:
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
except ArtifactLockBusy as exc:
|
||||
held.close()
|
||||
print(f"ERROR: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
all_ok = True
|
||||
total_start = time.monotonic()
|
||||
|
||||
@@ -18,7 +18,12 @@ 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, hold_artifact_lock # noqa: E402
|
||||
from common import ( # noqa: E402
|
||||
ArtifactLockBusy,
|
||||
artifact_lock,
|
||||
hold_artifact_lock,
|
||||
hold_repository_outputs,
|
||||
)
|
||||
|
||||
|
||||
def _tempdir(test: unittest.TestCase) -> str:
|
||||
@@ -200,6 +205,29 @@ class PackLockCliTest(unittest.TestCase):
|
||||
self.assertIn("is in use by another run", proc.stderr)
|
||||
|
||||
|
||||
class PipelineHoldsItsFixedOutputs(unittest.TestCase):
|
||||
"""A pipeline on a scratch --output-dir still rewrites database.json,
|
||||
install/, README.md and docs/: only the output directory was locked."""
|
||||
|
||||
def test_a_second_pipeline_is_refused_whatever_its_output_dir(self):
|
||||
scratch = _tempdir(self)
|
||||
with hold_repository_outputs(str(REPO_ROOT)):
|
||||
proc = subprocess.run(
|
||||
[sys.executable, "scripts/pipeline.py", "--offline", "--skip-docs",
|
||||
"--output-dir", scratch],
|
||||
capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=120,
|
||||
)
|
||||
self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
|
||||
self.assertIn("another pipeline run is writing database.json", proc.stderr)
|
||||
self.assertNotIn("generate database", proc.stdout.lower())
|
||||
|
||||
def test_two_holds_of_the_repository_exclude_each_other(self):
|
||||
with hold_repository_outputs(str(REPO_ROOT)), \
|
||||
self.assertRaises(ArtifactLockBusy):
|
||||
with hold_repository_outputs(str(REPO_ROOT)):
|
||||
pass
|
||||
|
||||
|
||||
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
|
||||
|
||||
+5
-2
@@ -88,8 +88,11 @@ alone, run `python scripts/check_buildbot_system.py`.
|
||||
rewrites nothing, so a diff on a generated file always means content.
|
||||
- **One run per output directory.** Pack generation takes a lock on
|
||||
`<output-dir>/.lock`, and a second run on the same folder stops at once
|
||||
rather than read a half-written ZIP. Experiments use a scratch
|
||||
`--output-dir` under `tmp/`.
|
||||
rather than read a half-written ZIP. Pack experiments with
|
||||
`generate_pack.py` use a scratch `--output-dir` under `tmp/`. A pipeline
|
||||
run also writes `database.json`, `install/`, `README.md` and `docs/` at
|
||||
fixed paths, so it holds the repository's `.lock` as well, and a second
|
||||
pipeline is refused whatever its `--output-dir`.
|
||||
- **Read the exit status before the output.** A generator that crashed leaves
|
||||
the previous artifact in place, which reads exactly like a run that changed
|
||||
nothing.
|
||||
|
||||
Reference in new issue
Block a user