From 064161cf3efe204c599503638d746dae2e983b8c Mon Sep 17 00:00:00 2001 From: Tobias Fischer Date: Sat, 10 Oct 2026 07:09:22 +1000 Subject: [PATCH] Small fixes from #39-#41; no upload tokens From Tim's PRs (the parts that fix real problems on main): - automerge goes by the label the PR has now (events can arrive out of order) and refuses to turn on without the bot token (a GITHUB_TOKEN merge doesn't start the build workflow) - CI installs with locked: the lock file must match pixi.toml - one build-branch generation per distribution at a time - check-patches includes shared/patch (a distribution's patches for a package replace the shared ones, like in vinca) - rs check accepts build_number 0 and an empty migration list; the pinning renders `migrations: []`; update-pinning fails when render-pinning does - import_distro: names follow the source distribution with --distro, files the old repository no longer has are removed, ci.yaml is kept Uploads use prefix.dev trusted publishing only: PREFIX_API_KEY and the anaconda.org upload (ANACONDA_API_TOKEN, also stripped from the generated workflows) are gone. Distributions whose channel is still on anaconda.org are read from there and upload to ROBOSTACK_UPLOAD_CHANNEL. Co-Authored-By: Tim de Jager Co-Authored-By: Claude Opus 5.5 --- .github/workflows/automerge.yaml | 16 +++++++++--- .github/workflows/bot.yaml | 4 +-- .github/workflows/main.yaml | 7 +++++- .github/workflows/testpr.yaml | 4 +-- README.md | 2 +- tools/check_patches_clean_apply.py | 15 +++++++---- tools/import_distro.py | 19 ++++++++++---- tools/maintenance.py | 13 +++++++--- tools/robostack.py | 40 ++++++++++++++---------------- 9 files changed, 76 insertions(+), 44 deletions(-) diff --git a/.github/workflows/automerge.yaml b/.github/workflows/automerge.yaml index dd5b64a6..7c6e95d0 100644 --- a/.github/workflows/automerge.yaml +++ b/.github/workflows/automerge.yaml @@ -19,6 +19,9 @@ permissions: jobs: automerge: if: github.event.label.name == 'automerge' + concurrency: + group: automerge-${{ github.event.pull_request.number }} + cancel-in-progress: false runs-on: ubuntu-latest steps: # The merge has to come from the bot (or a PAT): a merge made with the @@ -33,11 +36,18 @@ jobs: - name: Turn auto-merge on or off env: GH_TOKEN: ${{ steps.app.outputs.token || secrets.GHA_PAT || github.token }} + BOT_TOKEN: ${{ steps.app.outputs.token || secrets.GHA_PAT }} PR: ${{ github.event.pull_request.html_url }} - ACTION: ${{ github.event.action }} run: | - if [ "$ACTION" = labeled ]; then - gh pr merge "$PR" --auto --merge + # labeled/unlabeled events can arrive out of order: go by the label the + # pull request has now, not by the event + enabled=$(gh pr view "$PR" --json labels --jq 'any(.labels[]; .name == "automerge")') + if [ "$enabled" = true ]; then + if [ -z "$BOT_TOKEN" ]; then + echo "::error::auto-merge needs the bot app (ROBOSTACK_BOT_APP_ID + ROBOSTACK_BOT_PRIVATE_KEY) or GHA_PAT: a merge with the workflow token doesn't start the build workflow" + exit 1 + fi + GH_TOKEN="$BOT_TOKEN" gh pr merge "$PR" --auto --merge else gh pr merge "$PR" --disable-auto fi diff --git a/.github/workflows/bot.yaml b/.github/workflows/bot.yaml index 0321f599..05368fdb 100644 --- a/.github/workflows/bot.yaml +++ b/.github/workflows/bot.yaml @@ -61,7 +61,7 @@ jobs: run: git config --global maintenance.auto false && git config --global gc.auto 0 && git config --global core.longpaths true - uses: prefix-dev/setup-pixi@v0.11.0 with: - frozen: true + locked: true # the lock file must match pixi.toml pixi-version: v0.81.0 - id: plan env: @@ -113,7 +113,7 @@ jobs: run: git config --global maintenance.auto false && git config --global gc.auto 0 && git config --global core.longpaths true - uses: prefix-dev/setup-pixi@v0.11.0 with: - frozen: true + locked: true # the lock file must match pixi.toml pixi-version: v0.81.0 cache-write: false # the plan job saves the pixi environment cache - name: robostack-bot ${{ matrix.command }} ${{ matrix.distro }} diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index cc144bfe..1b8f7c56 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -50,6 +50,11 @@ jobs: needs: plan if: needs.plan.outputs.distros != '[]' runs-on: ubuntu-latest + # one generation per distribution at a time: two merges in quick succession must + # not push build branches in parallel (a newer pending run replaces an older one) + concurrency: + group: generate-${{ matrix.distro }} + cancel-in-progress: false strategy: fail-fast: false matrix: @@ -75,7 +80,7 @@ jobs: run: git config --global maintenance.auto false && git config --global gc.auto 0 && git config --global core.longpaths true - uses: prefix-dev/setup-pixi@v0.11.0 with: - frozen: true + locked: true # the lock file must match pixi.toml pixi-version: v0.81.0 # one job saves the shared pixi environment cache (same key for all) cache-write: ${{ matrix.distro == fromJSON(needs.plan.outputs.distros)[0] }} diff --git a/.github/workflows/testpr.yaml b/.github/workflows/testpr.yaml index 4d75786e..a80528b1 100644 --- a/.github/workflows/testpr.yaml +++ b/.github/workflows/testpr.yaml @@ -60,7 +60,7 @@ jobs: run: git config --global maintenance.auto false && git config --global gc.auto 0 && git config --global core.longpaths true - uses: prefix-dev/setup-pixi@v0.11.0 with: - frozen: true + locked: true # the lock file must match pixi.toml pixi-version: v0.81.0 cache-write: false # the build jobs save the pixi environment cache - name: Sanity checks (every distribution assembles, generated files up to date, sorted) @@ -98,7 +98,7 @@ jobs: run: git config --global maintenance.auto false && git config --global gc.auto 0 && git config --global core.longpaths true - uses: prefix-dev/setup-pixi@v0.11.0 with: - frozen: true + locked: true # the lock file must match pixi.toml pixi-version: v0.81.0 # every distribution's job on a platform has the same cache key; let one # of them save it instead of all racing ("Unable to reserve cache") diff --git a/README.md b/README.md index 4a6d2074..f1c0d616 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ of a distribution uses `full_rebuild: true` there. A new build number goes into - Those workflows build the packages and upload them to the distribution's channel. - **Repository variables:** - `ROBOSTACK_UPLOAD_CHANNEL`: upload to this prefix.dev channel instead (for example a test channel), or `none` to build without uploading. Use the canonical reference: `/` for a channel that isn't the namespace's primary one (e.g. `tobias-fischer/robostack-distro-test`). - - Upload credentials: prefix.dev channels use Repository Access (OIDC, no stored key). In the channel's *Settings > Repository Access*, authorize GitHub, this repository, workflow filename `build.yaml`, mode *Read/write*. The secret `PREFIX_API_KEY` is only needed for channels without Repository Access; while it is set, it is used instead. anaconda.org channels use `ANACONDA_API_TOKEN`. + - Upload credentials: prefix.dev channels use Repository Access (OIDC, no stored key). In the channel's *Settings > Repository Access*, authorize GitHub, this repository, workflow filename `build.yaml`, mode *Read/write*. There are no upload tokens: distributions whose channel is still on anaconda.org (`upload_target: anaconda`, read from there) can only upload to a prefix.dev channel set in `ROBOSTACK_UPLOAD_CHANNEL`. - `ROBOSTACK_BOT_APP_ID` with the secret `ROBOSTACK_BOT_PRIVATE_KEY`: the robostack-bot GitHub App. It pushes the build branches (which contain workflow files) and opens bot PRs so that CI runs on them. `GHA_PAT` works as a fallback. The app needs repository permissions Contents, Pull requests, Issues and Workflows (read and write), with its webhook inactive. diff --git a/tools/check_patches_clean_apply.py b/tools/check_patches_clean_apply.py index e9185daf..4eff250b 100644 --- a/tools/check_patches_clean_apply.py +++ b/tools/check_patches_clean_apply.py @@ -102,8 +102,10 @@ def parse_args() -> argparse.Namespace: ap.add_argument( "--patch-dir", type=Path, - default=PATCH_DIR, - help="Directory with the patches (default: ./patch)", + action="append", + help="Directory with the patches (default: ./patch); repeat it for layered " + "configurations, base first: like vinca, a later directory's patches for a " + "package replace the earlier ones", ) return ap.parse_args() @@ -368,8 +370,8 @@ def main() -> None: print("recipes/ folder not found – abort.") sys.exit(1) - patch_dir = args.patch_dir - if not patch_dir.is_dir(): + patch_dirs = args.patch_dir or [PATCH_DIR] + if not any(d.is_dir() for d in patch_dirs): print("patch/ folder not found – abort.") sys.exit(1) @@ -386,7 +388,10 @@ def main() -> None: if args.recipe: print(f"Selected {len(recipe_files)} recipe(s) via --recipe.") - all_patches = discover_all_patches(patch_dir) + all_patches = {} + for patch_dir in patch_dirs: + if patch_dir.is_dir(): + all_patches.update(discover_all_patches(patch_dir)) recreated = prepare_patch_recipes(recipe_files, all_patches) if not recreated: print("No recipes with patches found – nothing to test.") diff --git a/tools/import_distro.py b/tools/import_distro.py index 550f3ebc..3a2ff073 100644 --- a/tools/import_distro.py +++ b/tools/import_distro.py @@ -127,7 +127,10 @@ def copy_dir(self, path: str, dest: Path) -> None: def import_distro(src: Source, distro: str | None) -> str: ry = _ry() vinca = ry.load(src.text("vinca.yaml")) - distro = distro or vinca["ros_distro"] + # the old repository's names (ros--*) follow its distribution, also when + # --distro imports it under another name + source = vinca["ros_distro"] + distro = distro or source dest = rs.DISTROS / distro dest.mkdir(parents=True, exist_ok=True) @@ -165,11 +168,14 @@ def import_distro(src: Source, distro: str | None) -> str: # as they are (patch/dependencies.yaml is filtered below) shutil.rmtree(dest / "patch", ignore_errors=True) src.copy_dir("patch", dest) - for patch in (dest / "patch").glob(f"ros-{distro}-*.patch"): - patch.rename(patch.with_name("ros2-" + patch.name.removeprefix(f"ros-{distro}-"))) + for patch in (dest / "patch").glob(f"ros-{source}-*.patch"): + patch.rename(patch.with_name("ros2-" + patch.name.removeprefix(f"ros-{source}-"))) + # a file the old repository no longer has goes too (re-imports mirror it) for name in ("rosdistro_snapshot.yaml", "rosdistro_additional_recipes.yaml"): if (text := src.text(name)) is not None: (dest / name).write_text(text) + else: + (dest / name).unlink(missing_ok=True) # per-package files: drop entries identical to shared/ (keeping build numbers) for name, shared_file, target in ( @@ -178,8 +184,9 @@ def import_distro(src: Source, distro: str | None) -> str: ): text = src.text(name) if text is None: + target.unlink(missing_ok=True) continue - text = re.sub(rf"\bros-{distro}-(?=[a-z0-9])", "ros2-", text) + text = re.sub(rf"\bros-{source}-(?=[a-z0-9])", "ros2-", text) data = ry.load(text) or {} common = yaml.safe_load((rs.SHARED / shared_file).read_text()) or {} for key in list(data): @@ -218,7 +225,9 @@ def import_distro(src: Source, distro: str | None) -> str: "# (everything else is shared, see the README).\n" + yaml.safe_dump(settings, sort_keys=False, default_flow_style=None, width=100) ) - (dest / "ci.yaml").write_text((rs.ROOT / "tools" / "ci_default.yaml").read_text()) + # ci.yaml holds the distribution's current rebuild controls: keep them on re-imports + if not (dest / "ci.yaml").exists(): + (dest / "ci.yaml").write_text((rs.ROOT / "tools" / "ci_default.yaml").read_text()) return distro diff --git a/tools/maintenance.py b/tools/maintenance.py index 1f5132a1..91d1c56c 100644 --- a/tools/maintenance.py +++ b/tools/maintenance.py @@ -476,7 +476,8 @@ def update_pinning(distro: str) -> Result: before_cbc = cbc.read_text() if cbc.is_file() else "" print(f"Checking the dependencies of {distro} with the current pins", flush=True) conflicts_before = dependency_conflicts(distro) - rs.task(distro, "render-pinning", []) + if rs.task(distro, "render-pinning", []): + return Result(f"{distro}: pinning update failed", "`render-pinning` failed, see the log.", ok=False) added = sorted(set(migrations) - set(before[1])) candidates = {name for name, _ in reports} finished = sorted(set(before[1]) - set(migrations) - candidates) @@ -575,7 +576,7 @@ def new_distro(name: str, source: str) -> Result: "", "Next steps:", f"- [ ] review `distros/{name}/vinca.yaml` (mutex name/version, package selection)", - f"- [ ] create the `robostack-{name}` channel and its trusted publisher / upload token", + f"- [ ] create the `robostack-{name}` channel and its Repository Access (trusted publishing, see the README)", f"- [ ] port patches that still apply ({len(patches)} in `distros/{source}/patch/`), " f"check with `pixi run rs {name} check-patches`", ] @@ -769,7 +770,8 @@ def check() -> Result: sync = settings.get("rosdistro_sync") if sync is not None and sync != "manual" and not ((m := SYNC_TAG.match(str(sync))) and m["distro"] == distro): problems.append(f"{distro}: rosdistro_sync must be 'manual' or a sync tag like {distro}/2026-10-05") - if bool(settings.get("conda_forge_pinning_version")) != bool(settings.get("conda_forge_migrations")): + # an empty migration list is valid + if ("conda_forge_pinning_version" in settings) != ("conda_forge_migrations" in settings): problems.append(f"{distro}: set conda_forge_pinning_version and conda_forge_migrations together") try: vinca = rs.read_vinca(distro, "linux-64") @@ -778,7 +780,10 @@ def check() -> Result: continue if vinca.get("ros_distro") != distro: problems.append(f"{distro}: vinca.yaml has ros_distro {vinca.get('ros_distro')!r}") - for key in ("build_number", "mutex_package", "packages_select_by_deps"): + # build_number 0 is valid (new-distro starts there) + if not isinstance(vinca.get("build_number"), int) or vinca["build_number"] < 0: + problems.append(f"{distro}: vinca.yaml needs a build_number >= 0") + for key in ("mutex_package", "packages_select_by_deps"): if not vinca.get(key): problems.append(f"{distro}: vinca.yaml has no {key}") for patch in (d / "patch").glob("*.patch"): diff --git a/tools/robostack.py b/tools/robostack.py index 2dd515ba..46015061 100644 --- a/tools/robostack.py +++ b/tools/robostack.py @@ -105,8 +105,9 @@ def vinca_pinning(distro: str) -> str: f"# distros/{distro}/distro.yaml; edit those instead.", ""] if s.get("conda_forge_pinning_version"): out.append(f"conda_forge_pinning_version: {s['conda_forge_pinning_version']}") - out.append("migrations:") - out += [f" - {m}" for m in s.get("conda_forge_migrations") or []] + migrations = s.get("conda_forge_migrations") or [] + out.append("migrations:" if migrations else "migrations: []") + out += [f" - {m}" for m in migrations] else: out += [l for l in (SHARED / "pinning" / "conda_forge.yaml").read_text().splitlines() if not l.startswith("#")] @@ -217,28 +218,23 @@ def upload(distro: str, files: list[str], cwd: Path) -> int: ROBOSTACK_UPLOAD_CHANNEL (set by the staged build workflows from the repository variable of the same name) redirects uploads to another prefix.dev channel, e.g. a test channel (the builds then also read from it, see rattler_build); "none" - skips uploading. prefix.dev uploads use trusted publishing - (with an attestation), or the PREFIX_API_KEY secret if it is set. + skips uploading. prefix.dev uploads use trusted publishing (Repository Access, + OIDC; no stored key) and publish an attestation. """ override = os.environ.get("ROBOSTACK_UPLOAD_CHANNEL", "").strip() if override == "none": print(f"ROBOSTACK_UPLOAD_CHANNEL=none: not uploading {' '.join(files)}") return 0 s = settings(distro) - if override or s.get("upload_target", "prefix") == "prefix": - channel = override or s.get("channel_name", f"robostack-{distro}") - cmd = ["rattler-build", "upload", "prefix", "-c", channel, "--skip-existing"] - if not os.environ.get("PREFIX_API_KEY"): - # the build workflows pass the secret through; without it the variable is - # set but empty, and rattler-build would still take it for an API key - # ("--generate-attestation cannot be used with an API key") - os.environ.pop("PREFIX_API_KEY", None) - cmd.append("--generate-attestation") - else: - token = os.environ.get("ANACONDA_API_TOKEN", "") - cmd = ["rattler-build", "upload", "anaconda", "-o", s.get("channel_name", f"robostack-{distro}"), "-a", token, "--force"] - # Sigstore (attestations) and the channels fail now and then; uploads are - # idempotent (--skip-existing / --force), so retry. + if not override and s.get("upload_target", "prefix") != "prefix": + # anaconda.org has no trusted publishing, and we keep no tokens + print(f"{distro}'s channel {s.get('channel_name')} is on anaconda.org: uploads go to prefix.dev only " + "(set ROBOSTACK_UPLOAD_CHANNEL, or move the channel to prefix.dev)", file=sys.stderr) + return 1 + channel = override or s.get("channel_name", f"robostack-{distro}") + cmd = ["rattler-build", "upload", "prefix", "-c", channel, "--skip-existing", "--generate-attestation"] + # Sigstore (attestations) and the channel fail now and then; uploads are + # idempotent (--skip-existing), so retry. for attempt in range(1, 4): rc = run(cmd + files, cwd) if rc == 0 or attempt == 3: @@ -285,7 +281,9 @@ def task(distro: str, name: str, args: list[str]) -> int: if name == "check-orphaned-patches": return run(py + [str(TOOLS / "check_orphaned_platform_patches.py"), "--patch-dir", str(d / "patch"), *args], w) if name == "check-patches": - return run(py + [str(TOOLS / "check_patches_clean_apply.py"), "--patch-dir", str(d / "patch"), *args], w) + # shared/patch too, which the distribution's patches for the same package replace + dirs = ["--patch-dir", str(SHARED / "patch"), "--patch-dir", str(d / "patch")] + return run(py + [str(TOOLS / "check_patches_clean_apply.py"), *dirs, *args], w) if name == "check-deps": return run(py + [str(TOOLS / "check_dependency_compat.py"), *args], w) if name == "gap-report": @@ -325,6 +323,8 @@ def generate_gha(distro: str, args: list[str]) -> int: # 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] + # 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): raise SystemExit(f"{wf}: the Windows workflow doesn't use .scripts/build_win.bat") if any(l.startswith("env:") for l in lines): @@ -335,8 +335,6 @@ def generate_gha(distro: str, args: list[str]) -> int: " # optional repository variable: upload to this prefix.dev channel instead", " # (e.g. a test channel), or 'none' to only build", " ROBOSTACK_UPLOAD_CHANNEL: ${{ vars.ROBOSTACK_UPLOAD_CHANNEL }}", - " # optional secret: prefix.dev API key, for channels without Repository Access (OIDC)", - " PREFIX_API_KEY: ${{ secrets.PREFIX_API_KEY }}", ] # one file name on every build branch (each branch carries only its own # workflow), so a single prefix.dev Repository Access source matches them all