From aa05e5cebb581f0e135d2427c2f7db8ee26db47f Mon Sep 17 00:00:00 2001 From: Tobias Fischer Date: Sat, 10 Oct 2026 11:25:50 +1000 Subject: [PATCH] CI and tooling fixes from the audit (#25, #26, #33, #35) - ci_config.py: the ci.yaml cache controls (full_rebuild, evict_cache) apply once per configuration; a marker in the cache records them, so a retry of the same rebuild keeps what the earlier attempt built (#25) - generated build workflows: one run per distribution and platform at a time (concurrency group, not cancel-in-progress: re-running a failed job of an older run would cancel the newer one) (#26) - new-distro checks the source distribution first and removes the new directory if creating it fails, so the corrected command can run (#33) - rs build / build-one without generated recipes says to run generate-recipes instead of rattler-build's path error (#35) Co-Authored-By: Claude Opus 5.5 --- tools/ci_config.py | 20 ++++++++++++++++++++ tools/maintenance.py | 11 +++++++++++ tools/robostack.py | 12 +++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/tools/ci_config.py b/tools/ci_config.py index 7d750d43a..31c3ff031 100644 --- a/tools/ci_config.py +++ b/tools/ci_config.py @@ -9,18 +9,26 @@ - rosidl_generator_py # ROS name (dashes or underscores) - roboplan* # or a glob +The controls apply once per configuration: a marker in the cache records them, so a +retry of the same rebuild keeps the packages the earlier attempt built (a full +rebuild that takes longer than one runner's time limit can finish across retries). +Changing full_rebuild or evict_cache applies them again. + Each evict_cache entry matches both package name prefixes (``ros2-`` and ``ros--``, for builds made before ``package_name_mode: new``) and the plain name (packages that vinca built under a conda-forge name). """ import argparse +import json import shutil import sys from pathlib import Path import yaml +MARKER = ".ci-controls" + def main() -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) @@ -34,8 +42,18 @@ def main() -> int: config = yaml.safe_load(args.config.read_text()) or {} distro = yaml.safe_load(args.vinca.read_text())["ros_distro"] cache = args.cache_dir + controls = json.dumps({"full_rebuild": bool(config.get("full_rebuild", False)), + "evict_cache": [str(e) for e in config.get("evict_cache") or []]}, sort_keys=True) + marker = cache / MARKER if not cache.is_dir(): + # nothing restored: this attempt starts clean, its packages count for the retries print(f"No cache directory {cache}; nothing to do.") + cache.mkdir(parents=True) + marker.write_text(controls) + return 0 + + if marker.is_file() and marker.read_text() == controls: + print("ci.yaml: controls already applied to this cache (a retry); keeping its packages.") return 0 if config.get("full_rebuild", False): @@ -45,6 +63,7 @@ def main() -> int: shutil.rmtree(child) else: child.unlink() + marker.write_text(controls) return 0 for entry in config.get("evict_cache") or []: @@ -56,6 +75,7 @@ def main() -> int: for path in sorted(cache.glob(prefix + suffix)): print(f"ci.yaml: evicting {path.name}") path.unlink() + marker.write_text(controls) return 0 diff --git a/tools/maintenance.py b/tools/maintenance.py index 91d1c56c7..03203189d 100644 --- a/tools/maintenance.py +++ b/tools/maintenance.py @@ -549,7 +549,18 @@ def new_distro(name: str, source: str) -> Result: src, dest = rs.DISTROS / source, rs.DISTROS / name if dest.exists(): raise SystemExit(f"{dest} exists") + if not (src / "vinca.yaml").is_file(): + raise SystemExit(f"no distribution {source!r} to start from (distros/{source}/vinca.yaml)") dest.mkdir(parents=True) + try: + return _populate_new_distro(name, source, src, dest) + except BaseException: + # leave nothing behind, so the corrected command can run again + shutil.rmtree(dest, ignore_errors=True) + raise + + +def _populate_new_distro(name: str, source: str, src: Path, dest: Path) -> Result: settings = rs.settings(source) keep = {k: settings[k] for k in ("upload_target",) if k in settings} (dest / "distro.yaml").write_text( diff --git a/tools/robostack.py b/tools/robostack.py index 46015061e..c0ff4c158 100644 --- a/tools/robostack.py +++ b/tools/robostack.py @@ -288,6 +288,10 @@ def task(distro: str, name: str, args: list[str]) -> int: return run(py + [str(TOOLS / "check_dependency_compat.py"), *args], w) if name == "gap-report": return run(py + [str(TOOLS / "build_gap_report.py"), *args], w) + if name in ("build", "build-one") and not any((w / "recipes").glob("*/recipe.yaml")): + print(f"No recipes in {(w / 'recipes').relative_to(ROOT)}: run `pixi run rs {distro} generate-recipes` first", + file=sys.stderr) + return 1 if name == "build": return run(rattler_build(distro, ["--recipe-dir", "./recipes", *args]), w) if name == "build-ci": # used by .scripts/build_unix.sh / build_win.bat @@ -322,7 +326,13 @@ def generate_gha(distro: str, args: list[str]) -> int: lines = wf.read_text().splitlines() # all build branches share .github/workflows/build.yaml, which GitHub lists as one # workflow: run-name tells the runs apart - lines = [f"name: build\nrun-name: {distro} {ns.platform}" if l.startswith("name: ") else l for l in lines] + # one run per distribution and platform at a time: an older run can't upload + # alongside a newer one; a newer pending run replaces older pending ones (not + # cancel-in-progress: re-running a failed job of an older run would cancel the + # newer run) + lines = [f"name: build\nrun-name: {distro} {ns.platform}\n" + f"concurrency:\n group: build-{distro}-{ns.platform}\n cancel-in-progress: false" + if l.startswith("name: ") else l for l in lines] # uploads use prefix.dev trusted publishing only: no anaconda.org token lines = [l for l in lines if "ANACONDA_API_TOKEN" not in l] if ns.platform == "win-64" and not any("build-ci" in l for l in lines):