From 7879ed0414f9365db5bece0a5f8f63fd2736266d Mon Sep 17 00:00:00 2001 From: Leif Johansson Date: Thu, 22 Oct 2026 05:06:07 +0200 Subject: [PATCH 1/2] Integration branches: plan-generated, verified, built to GHCR, cross-referenced on the PRs actions/integration (sync, verify, report, comment), a reusable integration-build.yml that verifies before it builds and comments once per participating PR, a workflow template, and docker-build-push.yml gains a `target` input and a `digest` output (backward compatible). Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/docker-build-push.yml | 13 + .github/workflows/integration-build.yml | 142 ++++++ .github/workflows/integration-tests.yml | 29 ++ README.md | 6 +- actions/integration/action.yml | 54 ++ actions/integration/integration.py | 463 ++++++++++++++++++ actions/integration/tests/test_integration.py | 271 ++++++++++ docs/integration-branches.md | 73 +++ .../integration.properties.json | 6 + workflow-templates/integration.yml | 20 + 10 files changed, 1076 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/integration-build.yml create mode 100644 .github/workflows/integration-tests.yml create mode 100644 actions/integration/action.yml create mode 100644 actions/integration/integration.py create mode 100644 actions/integration/tests/test_integration.py create mode 100644 docs/integration-branches.md create mode 100644 workflow-templates/integration.properties.json create mode 100644 workflow-templates/integration.yml diff --git a/.github/workflows/docker-build-push.yml b/.github/workflows/docker-build-push.yml index 25741bc..db29b5c 100644 --- a/.github/workflows/docker-build-push.yml +++ b/.github/workflows/docker-build-push.yml @@ -154,10 +154,21 @@ on: type: boolean required: false default: false + target: + description: 'Dockerfile build target (docker build --target); empty builds the last stage' + type: string + required: false + default: '' + outputs: + digest: + description: 'Digest of the pushed image (sha256:...); empty when nothing was pushed' + value: ${{ jobs.build-and-push.outputs.digest }} jobs: build-and-push: runs-on: ubuntu-latest + outputs: + digest: ${{ steps.build.outputs.digest }} # Trust gate: run automatically for pushes, tags, dispatches, and same-repo PRs. # Fork PRs require the 'safe-to-build' label (only org members can apply it). # Fork PRs NEVER get images pushed, even with push-pr-images enabled. @@ -259,6 +270,7 @@ jobs: tags: ${{ steps.dispatch-tag.outputs.tags || steps.meta-custom.outputs.tags || steps.meta-standard.outputs.tags }} labels: ${{ steps.meta-custom.outputs.labels || steps.meta-standard.outputs.labels }} build-args: ${{ inputs.build-args }} + target: ${{ inputs.target }} cache-from: type=gha cache-to: type=gha,mode=max @@ -274,6 +286,7 @@ jobs: load: true tags: ${{ inputs.image-name }}:trivy-scan build-args: ${{ inputs.build-args }} + target: ${{ inputs.target }} cache-from: type=gha - name: Determine Trivy scan target diff --git a/.github/workflows/integration-build.yml b/.github/workflows/integration-build.yml new file mode 100644 index 0000000..85a61f9 --- /dev/null +++ b/.github/workflows/integration-build.yml @@ -0,0 +1,142 @@ +# Reusable workflow: build and publish an integration branch ("bag of PRs") - but only if it verifies. +# https://github.com/sirosfoundation/.github (docs/integration-branches.md) +# +# An integration/ branch is generated from .integration/plan.yaml. This workflow: +# 1. verifies the branch against its plan (a hand edit, patch or hidden change fails the build); glue is classified +# (resolution-only vs new code) in the log +# 2. builds and pushes ghcr.io/:- through docker-build-push.yml (Buildx, Trivy, cache) +# 3. posts/updates ONE comment on each participating PR (marker comment, edited in place): the branch, the image digest, the +# other PRs it was combined with. Skipped when the plan says `ephemeral: true`. +# +# Images contain UNMERGED code: never `latest`, pin consumers by DIGEST, re-pin to a release when the PRs land. +# +# Usage (see workflow-templates/integration.yml): +# +# on: +# push: +# branches: ['integration/**'] +# jobs: +# integration: +# uses: sirosfoundation/.github/.github/workflows/integration-build.yml@main +# permissions: +# contents: read +# packages: write +# security-events: write +# with: +# image-name: sirosfoundation/integration/my-service +# secrets: +# comment-token: ${{ secrets.INTEGRATION_COMMENT_TOKEN }} # optional; needed to comment on PRs in OTHER repos +name: Build Integration Branch + +on: + workflow_call: + inputs: + image-name: + description: 'GHCR image name without registry prefix (e.g. sirosfoundation/integration/vc-apigw)' + type: string + required: true + dockerfile: + type: string + required: false + default: './Dockerfile' + context: + type: string + required: false + default: '.' + build-args: + description: 'Docker build arguments (multiline KEY=VALUE)' + type: string + required: false + default: '' + target: + description: 'Dockerfile build target' + type: string + required: false + default: '' + platforms: + type: string + required: false + default: 'linux/amd64' + comment: + description: 'Comment on the participating PRs (ignored for an ephemeral plan)' + type: boolean + required: false + default: true + secrets: + comment-token: + description: 'Token allowed to comment on the PRs (needed for PRs in repositories other than this one)' + required: false + +permissions: + contents: read + +jobs: + verify: + runs-on: ubuntu-latest + outputs: + tag: ${{ steps.tag.outputs.tag }} + ephemeral: ${{ steps.plan.outputs.ephemeral }} + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Verify the branch against its plan + uses: sirosfoundation/.github/actions/integration@main + with: + command: verify + args: --tip HEAD + token: ${{ secrets.comment-token || github.token }} + + - name: Plan facts + id: plan + run: | + set -euo pipefail + if grep -Eq '^ephemeral: *true' .integration/plan.yaml; then echo "ephemeral=true" >> "$GITHUB_OUTPUT"; else echo "ephemeral=false" >> "$GITHUB_OUTPUT"; fi + + - name: Tag + id: tag + env: + REF_NAME: ${{ github.ref_name }} + run: | + set -euo pipefail + SLUG=$(echo "$REF_NAME" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9._-]/-/g') + echo "tag=${SLUG}-$(git rev-parse --short=7 HEAD)" >> "$GITHUB_OUTPUT" + + build: + needs: verify + permissions: + contents: read + packages: write + security-events: write + uses: sirosfoundation/.github/.github/workflows/docker-build-push.yml@main + with: + image-name: ${{ inputs.image-name }} + dockerfile: ${{ inputs.dockerfile }} + context: ${{ inputs.context }} + build-args: ${{ inputs.build-args }} + target: ${{ inputs.target }} + platforms: ${{ inputs.platforms }} + custom-ref: ${{ github.ref }} + custom-tag: ${{ needs.verify.outputs.tag }} + + comment: + needs: [verify, build] + if: inputs.comment && needs.verify.outputs.ephemeral != 'true' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Comment on the participating PRs + uses: sirosfoundation/.github/actions/integration@main + with: + command: comment + args: >- + --tip HEAD + --branch ${{ github.ref_name }} + --branch-repo ${{ github.repository }} + --image ghcr.io/${{ inputs.image-name }}@${{ needs.build.outputs.digest }} + --run-url ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + token: ${{ secrets.comment-token || github.token }} diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml new file mode 100644 index 0000000..ba270e4 --- /dev/null +++ b/.github/workflows/integration-tests.yml @@ -0,0 +1,29 @@ +name: Integration tool tests + +on: + push: + branches: [main] + paths: + - 'actions/integration/**' + - '.github/workflows/integration-*.yml' + - '.github/workflows/docker-build-push.yml' + pull_request: + paths: + - 'actions/integration/**' + - '.github/workflows/integration-*.yml' + - '.github/workflows/docker-build-push.yml' + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - run: python3 -m pip install --quiet pyyaml + - run: python3 -m unittest discover -s actions/integration/tests -v + - name: actionlint + run: | + bash <(curl -fsSL https://raw.githubusercontent.com/rhysd/actionlint/v1.7.7/scripts/download-actionlint.bash) 1.7.7 + ./actionlint -color .github/workflows/integration-build.yml .github/workflows/integration-tests.yml .github/workflows/docker-build-push.yml diff --git a/README.md b/README.md index 57f12e8..669e9ff 100644 --- a/README.md +++ b/README.md @@ -4,4 +4,8 @@ This repository contains organization-wide GitHub configuration files that are u ## Changes to this repo -Since changes here show up across every repo (unless they've set up their own templates), any changes here should be **well discussed and approved** before merging. \ No newline at end of file +Since changes here show up across every repo (unless they've set up their own templates), any changes here should be **well discussed and approved** before merging. + +## Integration branches + +Testing several unmerged PRs together? See [docs/integration-branches.md](docs/integration-branches.md): plan-generated `integration/**` branches, verified, built to GHCR, with a cross-reference comment on every participating PR. diff --git a/actions/integration/action.yml b/actions/integration/action.yml new file mode 100644 index 0000000..3bf4373 --- /dev/null +++ b/actions/integration/action.yml @@ -0,0 +1,54 @@ +name: integration +description: > + Integration branches ("bags of PRs"): sync, verify, report and comment. An integration/ branch is generated from + .integration/plan.yaml; verify proves nothing else is on it. See docs/integration-branches.md in sirosfoundation/.github. + +inputs: + command: + description: 'sync | verify | report | comment' + required: true + args: + description: 'Further arguments for integration.py (e.g. "--tip HEAD --summary")' + required: false + default: '' + path: + description: 'A clone of the repository holding the integration branch' + required: false + default: '.' + token: + description: 'GitHub token for the API (PR metadata, comments)' + required: false + default: '' + +outputs: + result: + description: 'stdout of the command' + value: ${{ steps.run.outputs.result }} + +runs: + using: composite + steps: + - name: Install PyYAML + shell: bash + run: python3 -m pip install --quiet pyyaml + + - name: integration.py + id: run + shell: bash + env: + GH_TOKEN: ${{ inputs.token }} + COMMAND: ${{ inputs.command }} + ARGS: ${{ inputs.args }} + REPO_PATH: ${{ inputs.path }} + ACTION_PATH: ${{ github.action_path }} + run: | + set -euo pipefail + # ARGS is split on whitespace on purpose (no quoting inside values); it comes from the calling workflow, not from users. + # shellcheck disable=SC2086 + OUT=$(python3 "$ACTION_PATH/integration.py" "$COMMAND" --path "$REPO_PATH" --https $ARGS) + { + echo "result<<__INTEGRATION_EOF__" + echo "$OUT" + echo "__INTEGRATION_EOF__" + } >> "$GITHUB_OUTPUT" + echo "$OUT" diff --git a/actions/integration/integration.py b/actions/integration/integration.py new file mode 100644 index 0000000..a62aab0 --- /dev/null +++ b/actions/integration/integration.py @@ -0,0 +1,463 @@ +#!/usr/bin/env python3 +"""Integration branches: a documented, reproducible "bag of PRs". + +An `integration/` branch is GENERATED from a plan that lives in the branch itself, `.integration/plan.yaml`: + + version: 1 + base: {repo: SUNET/vc, ref: main} + ephemeral: false # true: an ad-hoc test (no PR comments) + steps: + - merge: SUNET/vc#839 # a PR head, merged --no-ff; `@sha` pins the head + - rebase: {pr: SUNET/vc#831} # the PR's own commits cherry-picked onto the current tip (automatic only) + - glue: {branch: sirosfoundation/vc:glue/839-831, note: resolves seed_recover} # a branch resolving a conflict + locked: # written by `sync`: exactly what was built; `verify` replays these + base_sha: <40 hex> + steps: [{head: <40 hex>}, ...] + +Commands (all take --path, a clone of the repository holding the branch): + + sync --branch B --plan FILE|--from TIP (re)create the branch from a plan (fetches PR heads), lock it, commit the plan + verify --tip B regenerate from the locked plan and require the same tree; classify glue + report --tip B [--json|--summary] what is in it: base, the steps, PR metadata, glue classification + comment --tip B --image REF [--run-url U] one marker comment per PR, edited in place (--retire when the branch goes) + +`verify` is the guarantee: the tip's tree (ignoring `.integration/`) must equal the replay of the plan, so no hand edit, +patch commit or change hidden in a merge commit can be on the branch. A glue branch is the only place a conflict may be +resolved, and it is classified: RESOLUTION-ONLY (every added line already exists in one of the PRs or the base: no review +needed) or NEW CODE (lines found nowhere else: it needs a draft PR and review like any change). + +A branch without a plan (older ones) is still reported from its history (`report`), and `verify --legacy` checks it the old +way: only merges, none hand-edited. +""" +import argparse +import json +import re +import subprocess +import sys +import tempfile +from pathlib import Path + +PLAN = ".integration/plan.yaml" +MARKER = "" + + +def git(path, *args, check=True, input=None): + out = subprocess.run(["git", "-C", str(path), *args], capture_output=True, text=True, input=input) + if check and out.returncode != 0: + raise SystemExit(f"git {' '.join(args)}: {out.stderr.strip() or out.stdout.strip()}") + return out + + +def ssh_url(repo): + return f"git@github.com:{repo}.git" + + +def https_url(repo): + return f"https://github.com/{repo}.git" + + +# --- plan --------------------------------------------------------------------------------------------------------------- + +def parse_pr(spec): + m = re.fullmatch(r"([\w.-]+/[\w.-]+)#(\d+)(?:@([0-9a-f]{7,40}))?", str(spec)) + if not m: + raise SystemExit(f"not a PR reference (owner/repo#N[@sha]): {spec}") + return m.group(1), int(m.group(2)), m.group(3) + + +def parse_branch(spec): + m = re.fullmatch(r"([\w.-]+/[\w.-]+):(.+?)(?:@([0-9a-f]{7,40}))?", str(spec)) + if not m: + raise SystemExit(f"not a branch reference (owner/repo:branch[@sha]): {spec}") + return m.group(1), m.group(2), m.group(3) + + +def load_plan(text): + import yaml + plan = yaml.safe_load(text) + if not isinstance(plan, dict) or plan.get("version") != 1 or not plan.get("steps") or "base" not in plan: + raise SystemExit("not a version 1 integration plan (needs base and steps)") + for step in plan["steps"]: + kinds = [k for k in ("merge", "rebase", "glue") if k in step] + if len(kinds) != 1: + raise SystemExit(f"a step is exactly one of merge, rebase, glue: {step}") + return plan + + +def dump_plan(plan): + import yaml + return yaml.safe_dump(plan, sort_keys=False, width=140) + + +def step_kind(step): + return next(k for k in ("merge", "rebase", "glue") if k in step) + + +def step_ref(step): + """(kind, repo, ref-or-number, pinned sha) of a step.""" + kind = step_kind(step) + if kind == "merge": + repo, number, sha = parse_pr(step["merge"]) + return kind, repo, number, sha + if kind == "rebase": + repo, number, sha = parse_pr(step["rebase"]["pr"] if isinstance(step["rebase"], dict) else step["rebase"]) + return kind, repo, number, sha + repo, branch, sha = parse_branch(step["glue"]["branch"] if isinstance(step["glue"], dict) else step["glue"]) + return kind, repo, branch, sha + + +# --- replay ------------------------------------------------------------------------------------------------------------- + +def fetch_head(path, kind, repo, ref, url_for): + """Fetch the head of a PR (pull//head) or a branch into a private ref and return its sha.""" + name = f"refs/integration/{repo.replace('/', '_')}_{'pr' + str(ref) if kind != 'glue' else re.sub(r'[^\w.-]', '_', ref)}" + src = f"pull/{ref}/head" if kind != "glue" else f"refs/heads/{ref}" + fetched = git(path, "fetch", "-q", url_for(repo), f"+{src}:{name}", check=False) + if fetched.returncode != 0: + raise SystemExit(f"cannot fetch {repo} {src}: {fetched.stderr.strip()}") + return git(path, "rev-parse", name).stdout.strip() + + +def has_object(path, sha): + return git(path, "cat-file", "-e", f"{sha}^{{commit}}", check=False).returncode == 0 + + +def pr_commits(path, head, base_sha): + """The PR's own commits: everything reachable from its head and not from the base, oldest first, merges left out.""" + return git(path, "rev-list", "--reverse", "--no-merges", head, f"^{base_sha}").stdout.split() + + +def replay(path, plan, resolve, branch, base_sha, message=None): + """Build `branch` (checked out in `path`) at base_sha by running the plan. `resolve(i, step)` returns the head sha for step i. + Returns [{'step', 'kind', 'head', 'tip_before', 'novel_lines' (glue)}].""" + git(path, "checkout", "-q", "-B", branch, base_sha) + results = [] + heads = [resolve(i, step) for i, step in enumerate(plan["steps"])] + pr_heads = [h for h, step in zip(heads, plan["steps"]) if step_kind(step) != "glue"] + for i, step in enumerate(plan["steps"]): + kind, repo, ref, _ = step_ref(step) + head = heads[i] + before = git(path, "rev-parse", "HEAD").stdout.strip() + label = f"{repo}#{ref}" if kind != "glue" else f"{repo}:{ref}" + if kind == "rebase": + for sha in pr_commits(path, head, base_sha): + pick = git(path, "cherry-pick", "--allow-empty", "--keep-redundant-commits", sha, check=False) + if pick.returncode != 0: + git(path, "cherry-pick", "--abort", check=False) + raise SystemExit(f"rebase of {label}: {sha[:9]} does not apply automatically; a conflict needs a glue step, " + f"not a hand edit") + else: + text = f"Merge PR {label} @{head[:9]}" if kind == "merge" else f"Glue {label} @{head[:9]}" + note = step.get("note") or (step["glue"].get("note") if kind == "glue" and isinstance(step["glue"], dict) else None) + if note: + text += f": {note}" + merged = git(path, "merge", "--no-ff", "-m", text, head, check=False) + if merged.returncode != 0: + git(path, "merge", "--abort", check=False) + raise SystemExit(f"{label} @{head[:9]} conflicts with what is already on {branch}. A conflict is resolved on a " + f"glue branch that goes into the plan (a `glue` step before this one), never by hand: " + f"{merged.stdout.strip()[:300]}") + entry = {"step": i, "kind": kind, "head": head, "tip_before": before} + if kind == "glue": + entry["novel_lines"] = novel_lines(path, before, pr_heads, base_sha) + results.append(entry) + return results + + +def file_lines(path, rev, name): + out = git(path, "show", f"{rev}:{name}", check=False) + return set(out.stdout.splitlines()) if out.returncode == 0 else set() + + +def novel_lines(path, before, pr_heads, base_sha): + """Added lines of a glue step that exist in none of: the tip before it, the base, or ANY pull-request head of the plan (earlier + or later: a glue resolves a conflict with a PR that is merged after it). {file: [line, ...]}. Empty means RESOLUTION-ONLY. The + glue branch itself is deliberately not a source: its own lines are what is being judged.""" + after = git(path, "rev-parse", "HEAD").stdout.strip() + changed = git(path, "diff", "--name-only", before, after).stdout.split() + novel = {} + for name in changed: + added = [l[1:] for l in git(path, "diff", "-U0", before, after, "--", name).stdout.splitlines() + if l.startswith("+") and not l.startswith("+++")] + known = set() + for rev in [before, base_sha, *pr_heads]: + known |= file_lines(path, rev, name) + fresh = [l for l in added if l.strip() and l not in known] + if fresh: + novel[name] = fresh + return novel + + +def tree_without_plan(path, rev): + """The tree id of `rev` with `.integration/` removed.""" + with tempfile.NamedTemporaryFile() as idx: + env_git = ["git", "-C", str(path)] + import os + env = {**os.environ, "GIT_INDEX_FILE": idx.name} + subprocess.run([*env_git, "read-tree", rev], env=env, check=True, capture_output=True) + subprocess.run([*env_git, "rm", "-r", "-q", "--cached", "--ignore-unmatch", ".integration"], env=env, check=True, + capture_output=True) + return subprocess.run([*env_git, "write-tree"], env=env, check=True, capture_output=True, text=True).stdout.strip() + + +# --- sync / verify ------------------------------------------------------------------------------------------------------ + +def sync(path, branch, plan, url_for=ssh_url, push_to=None): + """(Re)create `branch` from `plan`: fetch every head, replay, lock the heads, commit the plan. Returns the locked plan.""" + path = Path(path) + if git(path, "status", "--porcelain").stdout.strip(): + raise SystemExit("the working tree is not clean") + base_repo, base_ref = plan["base"]["repo"], plan["base"]["ref"] + base_sha = fetch_head_ref(path, base_repo, base_ref, url_for) + heads = [] + + def resolve(i, step): + kind, repo, ref, pinned = step_ref(step) + head = fetch_head(path, kind, repo, ref, url_for) + if pinned and not head.startswith(pinned): + raise SystemExit(f"{repo} {ref} is {head[:9]}, not the pinned {pinned}: it has moved") + heads.append(head) + return head + + replay(path, plan, resolve, branch, base_sha) + locked = {**plan, "locked": {"base_sha": base_sha, "steps": [{"head": h} for h in heads]}} + (path / ".integration").mkdir(exist_ok=True) + (path / PLAN).write_text(dump_plan(locked)) + git(path, "add", PLAN) + git(path, "commit", "-q", "-m", "Integration plan (generated by integration.py sync; do not edit)") + if push_to: + git(path, "push", "--force-with-lease", push_to, f"{branch}:{branch}") + return locked + + +def fetch_head_ref(path, repo, ref, url_for): + name = f"refs/integration/base_{repo.replace('/', '_')}_{re.sub(r'[^\w.-]', '_', ref)}" + fetched = git(path, "fetch", "-q", url_for(repo), f"+refs/heads/{ref}:{name}", check=False) + if fetched.returncode != 0: + raise SystemExit(f"cannot fetch the base {repo}@{ref}: {fetched.stderr.strip()}") + return git(path, "rev-parse", name).stdout.strip() + + +def read_plan(path, tip): + out = git(path, "show", f"{tip}:{PLAN}", check=False) + if out.returncode != 0: + return None + plan = load_plan(out.stdout) + if "locked" not in plan: + raise SystemExit("the plan is not locked: run sync") + return plan + + +def verify(path, tip, url_for=ssh_url): + """Replay the locked plan in a scratch worktree and compare trees. Returns {'ok', 'problems', 'glue'}.""" + path = Path(path) + plan = read_plan(path, tip) + if plan is None: + return {"ok": False, "problems": [f"no {PLAN} on {tip}: a branch is generated from a plan"], "glue": []} + locked = plan["locked"] + tip_sha = git(path, "rev-parse", f"{tip}^{{commit}}").stdout.strip() + problems, glue = [], [] + work = Path(tempfile.mkdtemp()) + git(path, "worktree", "add", "-q", "--detach", str(work), tip_sha) + try: + def resolve(i, step): + head = locked["steps"][i]["head"] + if not has_object(work, head): # a head that is not in the branch's own history (a rebase step) + kind, repo, ref, _ = step_ref(step) + fetched = fetch_head(work, kind, repo, ref, url_for) + if fetched != head: + raise SystemExit(f"step {i}: the locked head {head[:9]} is no longer fetchable ({repo} {ref} is now {fetched[:9]})") + return head + results = replay(work, plan, resolve, "integration/verify-scratch", locked["base_sha"]) + replayed = tree_without_plan(work, "HEAD") + actual = tree_without_plan(path, tip_sha) + if replayed != actual: + diff = git(path, "diff", "--stat", replayed, actual).stdout.strip().splitlines()[-1:] or ["trees differ"] + problems.append("the branch is not what its plan produces (a hand edit, a patch or a change hidden in a merge): " + f"{diff[0].strip()}") + for r in results: + if r["kind"] == "glue": + step = plan["steps"][r["step"]] + glue.append({"step": r["step"], "branch": step_ref(step)[2], "head": r["head"], + "class": "new code" if r["novel_lines"] else "resolution-only", + "new_lines": sum(len(v) for v in r["novel_lines"].values()), "files": sorted(r["novel_lines"])}) + except SystemExit as e: + problems.append(str(e)) + finally: + git(path, "worktree", "remove", "--force", str(work), check=False) + git(path, "branch", "-D", "integration/verify-scratch", check=False) + return {"ok": not problems, "problems": problems, "glue": glue} + + +# --- legacy (no plan): derive from history ------------------------------------------------------------------------------ + +def legacy_problems(path, tip, base_ref): + """For a branch without a plan: every first-parent step must be a merge whose result is the automatic merge of its parents.""" + tip_sha = git(path, "rev-parse", "--verify", f"{tip}^{{commit}}").stdout.strip() + base = git(path, "merge-base", tip_sha, base_ref).stdout.strip() + out = [] + for line in git(path, "log", "--first-parent", "--reverse", "--format=%H|%P|%T|%s", f"{base}..{tip_sha}").stdout.splitlines(): + sha, parents, tree, subject = line.split("|", 3) + parents = parents.split() + if len(parents) != 2: + out.append(f"{sha[:9]} {subject[:60]}: not a merge of a pull request") + continue + replay_ = git(path, "merge-tree", "--write-tree", "--no-messages", *parents, check=False) + if replay_.returncode != 0: + out.append(f"{sha[:9]} {subject[:60]}: conflicts when replayed (resolved by hand inside the merge)") + elif replay_.stdout.split()[0] != tree: + out.append(f"{sha[:9]} {subject[:60]}: differs from the automatic merge of its parents") + return out + + +# --- report / comment --------------------------------------------------------------------------------------------------- + +def gh_json(args, run=subprocess.run): + out = run(["gh", "api", *args], capture_output=True, text=True) + if out.returncode != 0: + return None + try: + return json.loads(out.stdout) + except json.JSONDecodeError: + return None + + +def pr_info(repo, number, run=subprocess.run): + data = gh_json([f"repos/{repo}/pulls/{number}"], run) + if not data: + return {"state": "unknown", "title": "", "draft": False, "head": None, "labels": []} + return {"state": "merged" if data.get("merged_at") else data["state"], "title": data["title"], "draft": data.get("draft", False), + "head": data["head"]["sha"], "labels": [l["name"] for l in data.get("labels", [])]} + + +def report(path, tip, run=subprocess.run, with_api=True): + plan = read_plan(Path(path), tip) + if plan is None: + raise SystemExit(f"no {PLAN} on {tip}") + locked = plan["locked"] + steps = [] + for i, step in enumerate(plan["steps"]): + kind, repo, ref, _ = step_ref(step) + entry = {"kind": kind, "repo": repo, "ref": ref, "locked_head": locked["steps"][i]["head"]} + if kind != "glue" and with_api: + info = pr_info(repo, ref, run) + entry.update(info, head_moved=bool(info["head"]) and info["head"] != entry["locked_head"]) + if kind == "glue": + entry["note"] = step["glue"].get("note") if isinstance(step["glue"], dict) else None + steps.append(entry) + return {"base": plan["base"], "base_sha": locked["base_sha"], "ephemeral": bool(plan.get("ephemeral")), "steps": steps} + + +def comment_body(rep, branch_repo, branch, image, run_url, pr): + others = [f"{s['repo']}#{s['ref']}" for s in rep["steps"] if s["kind"] != "glue" and (s["repo"], s["ref"]) != pr] + glue = [f"`{s['repo']}:{s['ref']}`" for s in rep["steps"] if s["kind"] == "glue"] + lines = [MARKER.format(repo=branch_repo, branch=branch), + f"**Included in the integration branch `{branch_repo}:{branch}`** (a test build combining several unmerged PRs; " + "nothing here is a release).", + "", + f"- image: `{image}`" if image else "- image: (not built yet)", + f"- base: {rep['base']['repo']}@{rep['base']['ref']} `{rep['base_sha'][:9]}`"] + if others: + lines.append("- together with: " + ", ".join(others)) + if glue: + lines.append("- glue (conflict resolutions): " + ", ".join(glue)) + if run_url: + lines.append(f"- built by {run_url}") + lines += ["", "_Updated automatically; edited in place on every rebuild._"] + return "\n".join(lines) + + +def upsert_comment(repo, number, body, marker, run=subprocess.run): + comments = gh_json([f"repos/{repo}/issues/{number}/comments", "--paginate"], run) or [] + mine = next((c for c in comments if marker in (c.get("body") or "")), None) + if mine: + run(["gh", "api", "-X", "PATCH", f"repos/{repo}/issues/comments/{mine['id']}", "-f", f"body={body}"], capture_output=True, text=True) + return "updated" + run(["gh", "api", "-X", "POST", f"repos/{repo}/issues/{number}/comments", "-f", f"body={body}"], capture_output=True, text=True) + return "created" + + +def comment(path, tip, branch_repo, branch, image, run_url, retire=False, run=subprocess.run, dry_run=False): + rep = report(path, tip, run) + if rep["ephemeral"]: + return [("ephemeral", None)] + done = [] + for s in rep["steps"]: + if s["kind"] == "glue": + continue + body = comment_body(rep, branch_repo, branch, image, run_url, (s["repo"], s["ref"])) + if retire: + body = body.replace("**Included in the integration branch", "**RETIRED: was included in the integration branch") + marker = MARKER.format(repo=branch_repo, branch=branch) + done.append((f"{s['repo']}#{s['ref']}", "dry-run" if dry_run else upsert_comment(s["repo"], s["ref"], body, marker, run))) + return done + + +def summary(rep): + return ",".join(f"{s['repo']}#{s['ref']}@{s['locked_head'][:9]}" for s in rep["steps"] if s["kind"] != "glue") + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("command", choices=("sync", "verify", "report", "comment")) + ap.add_argument("--path", required=True) + ap.add_argument("--tip") + ap.add_argument("--branch") + ap.add_argument("--plan", help="sync: a plan file (without `locked`)") + ap.add_argument("--from", dest="source", help="sync: re-lock the plan of this existing branch at the PRs' latest heads") + ap.add_argument("--push", metavar="REMOTE") + ap.add_argument("--https", action="store_true", help="fetch over https instead of ssh") + ap.add_argument("--base", default="origin/main", help="verify --legacy: what a plan-less branch was built on") + ap.add_argument("--legacy", action="store_true") + ap.add_argument("--json", action="store_true") + ap.add_argument("--summary", action="store_true") + ap.add_argument("--image", default="") + ap.add_argument("--run-url", default="") + ap.add_argument("--branch-repo", default="") + ap.add_argument("--retire", action="store_true") + ap.add_argument("--dry-run", action="store_true") + args = ap.parse_args(argv) + path = Path(args.path) + url_for = https_url if args.https else ssh_url + if args.command == "sync": + if not args.branch: + raise SystemExit("sync needs --branch") + if args.plan: + plan = load_plan(Path(args.plan).read_text()) + elif args.source: + plan = read_plan(path, args.source) + else: + raise SystemExit("sync needs --plan or --from") + plan.pop("locked", None) + sync(path, args.branch, plan, url_for, args.push) + print(f"{args.branch}: {len(plan['steps'])} steps, locked") + return 0 + if not args.tip: + raise SystemExit(f"{args.command} needs --tip") + if args.command == "verify": + if args.legacy: + problems = legacy_problems(path, args.tip, args.base) + for p in problems: + print(f"DRIFT {p}", file=sys.stderr) + print("FAILED" if problems else "OK", file=sys.stderr) + return 1 if problems else 0 + result = verify(path, args.tip, url_for) + for p in result["problems"]: + print(f"DRIFT {p}", file=sys.stderr) + for g in result["glue"]: + print(f"glue {g['branch']}: {g['class']}" + (f" ({g['new_lines']} new lines in {', '.join(g['files'])})" if g["new_lines"] else ""), + file=sys.stderr) + print("OK" if result["ok"] else "FAILED", file=sys.stderr) + return 0 if result["ok"] else 1 + if args.command == "report": + rep = report(path, args.tip) + print(summary(rep) if args.summary else json.dumps(rep, indent=2) if args.json else __import__("yaml").safe_dump(rep, sort_keys=False, width=140)) + return 0 + for name, what in comment(path, args.tip, args.branch_repo or "?", args.branch or args.tip, args.image, args.run_url, args.retire, + dry_run=args.dry_run): + print(f"{name}: {what}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/actions/integration/tests/test_integration.py b/actions/integration/tests/test_integration.py new file mode 100644 index 0000000..f2906cf --- /dev/null +++ b/actions/integration/tests/test_integration.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 +"""actions/integration/integration.py against real git repositories (the "remotes" are local repos with refs/pull/N/head).""" +import json +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +import integration # noqa: E402 + +ENV = {**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@example.org", "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@example.org", "GIT_CONFIG_GLOBAL": "/dev/null", "GIT_CONFIG_SYSTEM": "/dev/null"} +os.environ.update({k: v for k, v in ENV.items() if k.startswith("GIT_")}) + + +class Remote: + """A repository holding `main`, PR heads under refs/pull/N/head and glue branches.""" + + def __init__(self): + self.dir = Path(tempfile.mkdtemp()) + self.git("init", "-q", "-b", "main") + self.write("a.txt", "base\n") + self.commit("base") + + def git(self, *args): + out = subprocess.run(["git", "-C", str(self.dir), *args], capture_output=True, text=True, env=ENV) + assert out.returncode == 0, f"git {args}: {out.stderr}" + return out.stdout.strip() + + def write(self, name, text): + (self.dir / name).write_text(text) + + def commit(self, msg): + self.git("add", "-A") + self.git("commit", "-q", "-m", msg) + return self.git("rev-parse", "HEAD") + + def pr(self, number, filename, text, base="main"): + self.git("checkout", "-q", "-B", f"pr{number}", base) + self.write(filename, text) + head = self.commit(f"feat: pr {number}") + self.git("update-ref", f"refs/pull/{number}/head", head) + self.git("checkout", "-q", "main") + return head + + def glue(self, name, parents, resolved): + """A branch that merges the given PR branches (conflicting) and commits `resolved` {file: text}.""" + self.git("checkout", "-q", "-B", name, f"pr{parents[0]}") + out = subprocess.run(["git", "-C", str(self.dir), "merge", "--no-commit", "--no-ff", f"pr{parents[1]}"], capture_output=True, text=True, env=ENV) + assert out.returncode != 0, "expected a conflict" + for f, t in resolved.items(): + self.write(f, t) + self.git("add", "-A") + self.git("commit", "-q", "-m", f"resolve {name}") + head = self.git("rev-parse", "HEAD") + self.git("checkout", "-q", "main") + return head + + +class Case(unittest.TestCase): + def setUp(self): + self.remote = Remote() + self.clone = Path(tempfile.mkdtemp()) + subprocess.run(["git", "clone", "-q", str(self.remote.dir), str(self.clone)], check=True, env=ENV) + self.url = lambda repo: str(self.remote.dir) + + def plan(self, steps, ephemeral=False): + return {"version": 1, "base": {"repo": "o/r", "ref": "main"}, "ephemeral": ephemeral, "steps": steps} + + def sync(self, steps, **kw): + return integration.sync(self.clone, "integration/x", self.plan(steps, **kw), self.url) + + def commit_on_branch(self, name, text): + (self.clone / name).write_text(text) + subprocess.run(["git", "-C", str(self.clone), "add", "-A"], check=True, env=ENV) + subprocess.run(["git", "-C", str(self.clone), "commit", "-q", "-m", "hand edit"], check=True, env=ENV) + + +class SyncAndVerify(Case): + def test_a_synced_branch_verifies_and_records_the_locked_heads(self): + h1 = self.remote.pr(1, "one.txt", "1\n") + h2 = self.remote.pr(2, "two.txt", "2\n") + locked = self.sync([{"merge": "o/r#1"}, {"merge": "o/r#2"}]) + self.assertEqual([s["head"] for s in locked["locked"]["steps"]], [h1, h2]) + result = integration.verify(self.clone, "integration/x", self.url) + self.assertEqual((result["ok"], result["problems"]), (True, [])) + + def test_a_patch_commit_on_the_branch_makes_verify_fail(self): + self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}]) + self.commit_on_branch("patch.txt", "not from any PR\n") + result = integration.verify(self.clone, "integration/x", self.url) + self.assertFalse(result["ok"]) + self.assertIn("not what its plan produces", result["problems"][0]) + + def test_a_change_hidden_inside_a_merge_commit_makes_verify_fail(self): + self.remote.pr(1, "one.txt", "1\n") + self.remote.pr(2, "two.txt", "2\n") + self.sync([{"merge": "o/r#1"}, {"merge": "o/r#2"}]) + # rewrite the history: amend the last merge to smuggle a file in, then re-commit the plan on top + c = self.clone + run = lambda *a: subprocess.run(["git", "-C", str(c), *a], check=True, env=ENV, capture_output=True, text=True) + plan_text = run("show", f"HEAD:{integration.PLAN}").stdout + run("reset", "-q", "--hard", "HEAD~1") + (c / "smuggled.txt").write_text("hidden\n") + run("add", "-A") + run("commit", "-q", "--amend", "--no-edit") + (c / ".integration").mkdir(exist_ok=True) + (c / integration.PLAN).write_text(plan_text) + run("add", "-A") + run("commit", "-q", "-m", "plan") + self.assertFalse(integration.verify(c, "integration/x", self.url)["ok"]) + + def test_verify_replays_the_locked_heads_even_if_the_pr_has_since_moved(self): + h1 = self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}]) + self.remote.git("checkout", "-q", "pr1") + self.remote.write("one.txt", "1b\n") + moved = self.remote.commit("feat: pr 1 moved") + self.remote.git("update-ref", "refs/pull/1/head", moved) + self.assertEqual(integration.verify(self.clone, "integration/x", self.url)["ok"], True) + self.assertNotEqual(h1, moved) + + def test_a_pinned_head_that_moved_is_refused(self): + self.remote.pr(1, "one.txt", "1\n") + with self.assertRaises(SystemExit) as cm: + self.sync([{"merge": "o/r#1@" + "0" * 9}]) + self.assertIn("has moved", str(cm.exception)) + + def test_a_branch_without_a_plan_fails_verify(self): + subprocess.run(["git", "-C", str(self.clone), "checkout", "-q", "-b", "integration/old"], check=True, env=ENV) + result = integration.verify(self.clone, "integration/old", self.url) + self.assertFalse(result["ok"]) + self.assertIn("generated from a plan", result["problems"][0]) + + def test_from_relocks_an_existing_plan_at_the_latest_heads(self): + self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}]) + self.remote.git("checkout", "-q", "pr1") + self.remote.write("one.txt", "1b\n") + newer = self.remote.commit("feat: pr 1 moved") + self.remote.git("update-ref", "refs/pull/1/head", newer) + plan = integration.read_plan(self.clone, "integration/x") + plan.pop("locked") + self.clone_run("checkout", "-q", "main") + locked = integration.sync(self.clone, "integration/y", plan, self.url) + self.assertEqual(locked["locked"]["steps"][0]["head"], newer) + + def clone_run(self, *a): + subprocess.run(["git", "-C", str(self.clone), *a], check=True, env=ENV, capture_output=True) + + +class Conflicts(Case): + def setUp(self): + super().setUp() + self.remote.pr(1, "a.txt", "one\n") + self.remote.pr(2, "a.txt", "two\n") + + def test_a_conflict_stops_the_sync_and_demands_a_glue_step(self): + with self.assertRaises(SystemExit) as cm: + self.sync([{"merge": "o/r#1"}, {"merge": "o/r#2"}]) + self.assertIn("glue", str(cm.exception)) + self.assertEqual(subprocess.run(["git", "-C", str(self.clone), "status", "--porcelain"], capture_output=True, text=True).stdout, "") + + def test_a_glue_that_only_chooses_between_existing_lines_is_resolution_only(self): + self.remote.glue("glue/1-2", [1, 2], {"a.txt": "one\ntwo\n"}) + self.sync([{"merge": "o/r#1"}, {"glue": {"branch": "o/r:glue/1-2", "note": "keep both"}}, {"merge": "o/r#2"}]) + result = integration.verify(self.clone, "integration/x", self.url) + self.assertTrue(result["ok"], result["problems"]) + self.assertEqual([(g["class"], g["new_lines"]) for g in result["glue"]], [("resolution-only", 0)]) + + def test_a_glue_that_adds_a_line_found_nowhere_else_is_new_code(self): + self.remote.glue("glue/1-2", [1, 2], {"a.txt": "one\ntwo\nbrand new behaviour\n"}) + self.sync([{"merge": "o/r#1"}, {"glue": "o/r:glue/1-2"}, {"merge": "o/r#2"}]) + result = integration.verify(self.clone, "integration/x", self.url) + self.assertTrue(result["ok"], "it reproduces; the point is that it is classified") + self.assertEqual((result["glue"][0]["class"], result["glue"][0]["new_lines"], result["glue"][0]["files"]), + ("new code", 1, ["a.txt"])) + + +class Rebase(Case): + def test_a_rebase_step_applies_the_prs_commits_onto_the_tip(self): + self.remote.pr(1, "one.txt", "1\n") + self.remote.pr(2, "two.txt", "2\n") + self.sync([{"merge": "o/r#1"}, {"rebase": {"pr": "o/r#2"}}]) + self.assertTrue((self.clone / "two.txt").exists()) + self.assertTrue(integration.verify(self.clone, "integration/x", self.url)["ok"]) + + def test_a_rebase_that_does_not_apply_cleanly_is_refused(self): + self.remote.pr(1, "a.txt", "one\n") + self.remote.pr(2, "a.txt", "two\n") + with self.assertRaises(SystemExit) as cm: + self.sync([{"merge": "o/r#1"}, {"rebase": {"pr": "o/r#2"}}]) + self.assertIn("glue step", str(cm.exception)) + + +class Report(Case): + def test_report_comes_from_the_plan_and_needs_no_api(self): + h = self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}]) + rep = integration.report(self.clone, "integration/x", with_api=False) + self.assertEqual(integration.summary(rep), f"o/r#1@{h[:9]}") + self.assertEqual(rep["steps"][0]["kind"], "merge") + + def test_legacy_verify_still_flags_a_plain_commit(self): + self.remote.pr(1, "one.txt", "1\n") + self.clone_run("checkout", "-q", "-b", "integration/old", "origin/main") + self.clone_run("fetch", "-q", str(self.remote.dir), "+pull/1/head:refs/p1") + self.clone_run("merge", "-q", "--no-ff", "-m", "Merge pr 1", "refs/p1") + self.assertEqual(integration.legacy_problems(self.clone, "integration/old", "origin/main"), []) + self.commit_on_branch("patch.txt", "x\n") + self.assertEqual(len(integration.legacy_problems(self.clone, "integration/old", "origin/main")), 1) + + def clone_run(self, *a): + subprocess.run(["git", "-C", str(self.clone), *a], check=True, env=ENV, capture_output=True) + + +class Comments(Case): + def fake_gh(self, existing): + calls = [] + + def run(cmd, **kw): + calls.append(cmd) + if "comments" in cmd[-2:] or (len(cmd) > 2 and cmd[2].endswith("/comments")) or "--paginate" in cmd: + return subprocess.CompletedProcess(cmd, 0, stdout=json.dumps(existing), stderr="") + if any("/pulls/" in c for c in cmd): + return subprocess.CompletedProcess(cmd, 0, stdout=json.dumps({"state": "open", "title": "t", "draft": False, + "head": {"sha": "a" * 40}, "labels": []}), stderr="") + return subprocess.CompletedProcess(cmd, 0, stdout="{}", stderr="") + return run, calls + + def test_one_marker_comment_per_pr_is_created_then_edited_in_place(self): + self.remote.pr(1, "one.txt", "1\n") + self.remote.pr(2, "two.txt", "2\n") + self.sync([{"merge": "o/r#1"}, {"merge": "o/r#2"}]) + marker = integration.MARKER.format(repo="o/b", branch="integration/x") + run, calls = self.fake_gh([]) + done = integration.comment(self.clone, "integration/x", "o/b", "integration/x", "ghcr.io/o/i@sha256:abc", "https://run", run=run) + self.assertEqual([d[1] for d in done], ["created", "created"]) + posts = [c for c in calls if "POST" in c] + self.assertEqual(len(posts), 2) + body = next(a for a in posts[0] if a.startswith("body=")) + self.assertIn(marker, body) + self.assertIn("ghcr.io/o/i@sha256:abc", body) + self.assertIn("together with: o/r#2", body) + run2, calls2 = self.fake_gh([{"id": 9, "body": marker + " old"}]) + done2 = integration.comment(self.clone, "integration/x", "o/b", "integration/x", "ghcr.io/o/i@sha256:def", "", run=run2) + self.assertEqual([d[1] for d in done2], ["updated", "updated"]) + self.assertTrue(any("PATCH" in c and "issues/comments/9" in " ".join(c) for c in calls2)) + + def test_an_ephemeral_plan_posts_nothing(self): + self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}], ephemeral=True) + run, calls = self.fake_gh([]) + self.assertEqual(integration.comment(self.clone, "integration/x", "o/b", "integration/x", "img", "", run=run), [("ephemeral", None)]) + self.assertFalse([c for c in calls if "POST" in c or "PATCH" in c]) + + def test_retire_marks_the_comment(self): + self.remote.pr(1, "one.txt", "1\n") + self.sync([{"merge": "o/r#1"}]) + run, calls = self.fake_gh([]) + integration.comment(self.clone, "integration/x", "o/b", "integration/x", "", "", retire=True, run=run) + body = next(a for c in calls if "POST" in c for a in c if a.startswith("body=")) + self.assertIn("RETIRED", body) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/integration-branches.md b/docs/integration-branches.md new file mode 100644 index 0000000..f0037a7 --- /dev/null +++ b/docs/integration-branches.md @@ -0,0 +1,73 @@ +# Integration branches: a documented, reproducible "bag of PRs" + +We often test a build that combines several unmerged pull requests. The rule that keeps such a build honest: + +> An `integration/` branch is **generated from a plan** and contains nothing else: no hand edits, no patch commits, no +> change hidden in a merge commit. Anything needed to make PRs fit together lives on a visible **glue branch** named in the plan. + +The plan is `.integration/plan.yaml` in the branch itself, so the branch is self-describing and an ad-hoc test needs no central +registry. `actions/integration/integration.py` creates, verifies and reports on branches; `integration-build.yml` builds them. + +## The plan + +```yaml +version: 1 +base: {repo: SUNET/vc, ref: main} +ephemeral: false # true: an ad-hoc test; no comments on the PRs +steps: + - merge: SUNET/vc#839 # merge the PR head (--no-ff); owner/repo#N[@sha] (the sha pins the head) + - glue: {branch: sirosfoundation/vc:glue/839-831, note: resolves the seed_recover conflict} + - merge: SUNET/vc#831 + - rebase: {pr: SUNET/vc#832} # the PR's own commits cherry-picked onto the tip (must apply cleanly) +locked: # written by `sync`: exactly what was built + base_sha: <40 hex> + steps: [{head: <40 hex>}, ...] +``` + +`sync` fetches every head, replays the steps, writes `locked` and commits the plan as the last commit. `verify` replays the +**locked** heads (so a PR that has moved since does not change the answer) and requires the branch's tree, ignoring `.integration/`, +to be identical. Any hand edit, patch commit, hand-resolved conflict or change smuggled into a merge shows up as a tree difference. + +## Conflicts and glue + +A conflict is never resolved inside a merge. It is resolved on a **glue branch** (a normal branch in one of our repos that merges +the conflicting PRs and commits the resolution), and the plan names it in a `glue` step before the PR it conflicts with. +`verify` classifies every glue branch: + +- **resolution-only**: every added line already exists in the tip before it, the base or one of the plan's PR heads. It only + chooses between existing code, so it needs no review and no PR. It is listed in the PR comments as "no new lines". +- **new code**: it adds lines found nowhere else. It is a change like any other: give it a draft PR and have it reviewed. + `verify` prints `glue : new code (N new lines in )`. A plan marked `ephemeral: true` may skip the PR. + +## Using it + +```sh +# create or update a branch from a plan (a re-lock at the latest heads: --from ) +python3 actions/integration/integration.py sync --path --branch integration/my-test --plan plan.yaml [--push origin] + +python3 actions/integration/integration.py verify --path --tip integration/my-test +python3 actions/integration/integration.py report --path --tip integration/my-test [--json|--summary] +``` + +Add the workflow template **Integration branch (bag of PRs)** to a repository (Actions -> New workflow). On every push to +`integration/**` it: +1. runs `verify` (a branch that fails is not built); +2. builds and pushes `ghcr.io/:-` via `docker-build-push.yml` (Buildx, cache, Trivy); +3. posts or updates **one comment per participating PR**, marked `` and edited in place: the + branch, the image digest, the other PRs it was combined with, the glue branches, the run URL. The links create backlinks on the + PRs both ways. Nothing is posted for an `ephemeral` plan. + +Commenting on a PR in another repository (an upstream) needs the `INTEGRATION_COMMENT_TOKEN` secret: a token of a bot account (or app) +that may comment there. Without it the comment step uses the workflow's own `GITHUB_TOKEN`, which can comment only in this repository. + +## Rules for consumers +These images contain UNMERGED code. Never use `latest`; pin by **digest** (the build summary prints it); re-pin to a release when the PRs +land. `report --json` gives each PR's state and whether its head moved since the build (the branch is then stale). + +## Limits (v1) +- The comments are not retired when the branch is deleted (the plan is gone with it); `comment --retire` works while it exists. +- A rebase step needs its locked head to still be fetchable (a force-pushed-away PR head cannot be replayed); merge and glue steps always can, + because their heads are in the branch's own history. +- Fetching other repositories' PR heads is over https: public repositories only. +- Branches created before this existed have no plan: `verify --legacy` checks them the old way (only merges, none hand-edited) and + the first run usually shows plain commits and hand-resolved conflicts. Regenerate them from a plan. diff --git a/workflow-templates/integration.properties.json b/workflow-templates/integration.properties.json new file mode 100644 index 0000000..67fa80c --- /dev/null +++ b/workflow-templates/integration.properties.json @@ -0,0 +1,6 @@ +{ + "name": "Integration branch (bag of PRs)", + "description": "Verify an integration/** branch against its plan, publish an image to GHCR and cross-reference the participating PRs.", + "iconName": "docker", + "categories": ["Docker", "continuous-integration"] +} diff --git a/workflow-templates/integration.yml b/workflow-templates/integration.yml new file mode 100644 index 0000000..32bb7df --- /dev/null +++ b/workflow-templates/integration.yml @@ -0,0 +1,20 @@ +# Builds an integration branch ("bag of PRs") when integration/** is pushed - and only if it verifies against its plan +# (.integration/plan.yaml). Publishes ghcr.io/:- and comments on the participating PRs. +# See https://github.com/sirosfoundation/.github/blob/main/docs/integration-branches.md +name: Integration + +on: + push: + branches: ['integration/**'] + +jobs: + integration: + uses: sirosfoundation/.github/.github/workflows/integration-build.yml@main + permissions: + contents: read + packages: write + security-events: write + with: + image-name: sirosfoundation/integration/${{ github.event.repository.name }} + secrets: + comment-token: ${{ secrets.INTEGRATION_COMMENT_TOKEN }} From 8a1ca48c21005ab710f66b221e24635df7a74ec6 Mon Sep 17 00:00:00 2001 From: Leif Johansson Date: Thu, 22 Oct 2026 05:59:47 +0200 Subject: [PATCH 2/2] integration: park the PR comments (follow-up); the base may be a commit sha The core (plan, sync, verify, glue classification, build) is validated first. The comment command, job, secret and tests are removed from this PR. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/integration-build.yml | 45 +------------ actions/integration/integration.py | 65 ++----------------- actions/integration/tests/test_integration.py | 48 -------------- docs/integration-branches.md | 13 ++-- workflow-templates/integration.yml | 4 +- 5 files changed, 13 insertions(+), 162 deletions(-) diff --git a/.github/workflows/integration-build.yml b/.github/workflows/integration-build.yml index 85a61f9..73f49b6 100644 --- a/.github/workflows/integration-build.yml +++ b/.github/workflows/integration-build.yml @@ -5,8 +5,7 @@ # 1. verifies the branch against its plan (a hand edit, patch or hidden change fails the build); glue is classified # (resolution-only vs new code) in the log # 2. builds and pushes ghcr.io/:- through docker-build-push.yml (Buildx, Trivy, cache) -# 3. posts/updates ONE comment on each participating PR (marker comment, edited in place): the branch, the image digest, the -# other PRs it was combined with. Skipped when the plan says `ephemeral: true`. +# (Cross-reference comments on the participating PRs are a follow-up, deliberately not part of this version.) # # Images contain UNMERGED code: never `latest`, pin consumers by DIGEST, re-pin to a release when the PRs land. # @@ -24,8 +23,6 @@ # security-events: write # with: # image-name: sirosfoundation/integration/my-service -# secrets: -# comment-token: ${{ secrets.INTEGRATION_COMMENT_TOKEN }} # optional; needed to comment on PRs in OTHER repos name: Build Integration Branch on: @@ -57,16 +54,6 @@ on: type: string required: false default: 'linux/amd64' - comment: - description: 'Comment on the participating PRs (ignored for an ephemeral plan)' - type: boolean - required: false - default: true - secrets: - comment-token: - description: 'Token allowed to comment on the PRs (needed for PRs in repositories other than this one)' - required: false - permissions: contents: read @@ -75,7 +62,6 @@ jobs: runs-on: ubuntu-latest outputs: tag: ${{ steps.tag.outputs.tag }} - ephemeral: ${{ steps.plan.outputs.ephemeral }} steps: - uses: actions/checkout@v7 with: @@ -86,13 +72,7 @@ jobs: with: command: verify args: --tip HEAD - token: ${{ secrets.comment-token || github.token }} - - - name: Plan facts - id: plan - run: | - set -euo pipefail - if grep -Eq '^ephemeral: *true' .integration/plan.yaml; then echo "ephemeral=true" >> "$GITHUB_OUTPUT"; else echo "ephemeral=false" >> "$GITHUB_OUTPUT"; fi + token: ${{ github.token }} - name: Tag id: tag @@ -119,24 +99,3 @@ jobs: platforms: ${{ inputs.platforms }} custom-ref: ${{ github.ref }} custom-tag: ${{ needs.verify.outputs.tag }} - - comment: - needs: [verify, build] - if: inputs.comment && needs.verify.outputs.ephemeral != 'true' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - with: - fetch-depth: 0 - - - name: Comment on the participating PRs - uses: sirosfoundation/.github/actions/integration@main - with: - command: comment - args: >- - --tip HEAD - --branch ${{ github.ref_name }} - --branch-repo ${{ github.repository }} - --image ghcr.io/${{ inputs.image-name }}@${{ needs.build.outputs.digest }} - --run-url ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - token: ${{ secrets.comment-token || github.token }} diff --git a/actions/integration/integration.py b/actions/integration/integration.py index a62aab0..d3a9722 100644 --- a/actions/integration/integration.py +++ b/actions/integration/integration.py @@ -5,7 +5,7 @@ version: 1 base: {repo: SUNET/vc, ref: main} - ephemeral: false # true: an ad-hoc test (no PR comments) + ephemeral: false # true: an ad-hoc test (reserved: used by the comments follow-up) steps: - merge: SUNET/vc#839 # a PR head, merged --no-ff; `@sha` pins the head - rebase: {pr: SUNET/vc#831} # the PR's own commits cherry-picked onto the current tip (automatic only) @@ -19,7 +19,6 @@ sync --branch B --plan FILE|--from TIP (re)create the branch from a plan (fetches PR heads), lock it, commit the plan verify --tip B regenerate from the locked plan and require the same tree; classify glue report --tip B [--json|--summary] what is in it: base, the steps, PR metadata, glue classification - comment --tip B --image REF [--run-url U] one marker comment per PR, edited in place (--retire when the branch goes) `verify` is the guarantee: the tip's tree (ignoring `.integration/`) must equal the replay of the plan, so no hand edit, patch commit or change hidden in a merge commit can be on the branch. A glue branch is the only place a conflict may be @@ -38,7 +37,6 @@ from pathlib import Path PLAN = ".integration/plan.yaml" -MARKER = "" def git(path, *args, check=True, input=None): @@ -231,8 +229,10 @@ def resolve(i, step): def fetch_head_ref(path, repo, ref, url_for): + """The base: a branch name, or a full 40-hex commit (a reproducible base).""" name = f"refs/integration/base_{repo.replace('/', '_')}_{re.sub(r'[^\w.-]', '_', ref)}" - fetched = git(path, "fetch", "-q", url_for(repo), f"+refs/heads/{ref}:{name}", check=False) + src = ref if re.fullmatch(r"[0-9a-f]{40}", ref) else f"refs/heads/{ref}" + fetched = git(path, "fetch", "-q", url_for(repo), f"+{src}:{name}", check=False) if fetched.returncode != 0: raise SystemExit(f"cannot fetch the base {repo}@{ref}: {fetched.stderr.strip()}") return git(path, "rev-parse", name).stdout.strip() @@ -310,7 +310,7 @@ def legacy_problems(path, tip, base_ref): return out -# --- report / comment --------------------------------------------------------------------------------------------------- +# --- report ------------------------------------------------------------------------------------------------------------- def gh_json(args, run=subprocess.run): out = run(["gh", "api", *args], capture_output=True, text=True) @@ -348,58 +348,13 @@ def report(path, tip, run=subprocess.run, with_api=True): return {"base": plan["base"], "base_sha": locked["base_sha"], "ephemeral": bool(plan.get("ephemeral")), "steps": steps} -def comment_body(rep, branch_repo, branch, image, run_url, pr): - others = [f"{s['repo']}#{s['ref']}" for s in rep["steps"] if s["kind"] != "glue" and (s["repo"], s["ref"]) != pr] - glue = [f"`{s['repo']}:{s['ref']}`" for s in rep["steps"] if s["kind"] == "glue"] - lines = [MARKER.format(repo=branch_repo, branch=branch), - f"**Included in the integration branch `{branch_repo}:{branch}`** (a test build combining several unmerged PRs; " - "nothing here is a release).", - "", - f"- image: `{image}`" if image else "- image: (not built yet)", - f"- base: {rep['base']['repo']}@{rep['base']['ref']} `{rep['base_sha'][:9]}`"] - if others: - lines.append("- together with: " + ", ".join(others)) - if glue: - lines.append("- glue (conflict resolutions): " + ", ".join(glue)) - if run_url: - lines.append(f"- built by {run_url}") - lines += ["", "_Updated automatically; edited in place on every rebuild._"] - return "\n".join(lines) - - -def upsert_comment(repo, number, body, marker, run=subprocess.run): - comments = gh_json([f"repos/{repo}/issues/{number}/comments", "--paginate"], run) or [] - mine = next((c for c in comments if marker in (c.get("body") or "")), None) - if mine: - run(["gh", "api", "-X", "PATCH", f"repos/{repo}/issues/comments/{mine['id']}", "-f", f"body={body}"], capture_output=True, text=True) - return "updated" - run(["gh", "api", "-X", "POST", f"repos/{repo}/issues/{number}/comments", "-f", f"body={body}"], capture_output=True, text=True) - return "created" - - -def comment(path, tip, branch_repo, branch, image, run_url, retire=False, run=subprocess.run, dry_run=False): - rep = report(path, tip, run) - if rep["ephemeral"]: - return [("ephemeral", None)] - done = [] - for s in rep["steps"]: - if s["kind"] == "glue": - continue - body = comment_body(rep, branch_repo, branch, image, run_url, (s["repo"], s["ref"])) - if retire: - body = body.replace("**Included in the integration branch", "**RETIRED: was included in the integration branch") - marker = MARKER.format(repo=branch_repo, branch=branch) - done.append((f"{s['repo']}#{s['ref']}", "dry-run" if dry_run else upsert_comment(s["repo"], s["ref"], body, marker, run))) - return done - - def summary(rep): return ",".join(f"{s['repo']}#{s['ref']}@{s['locked_head'][:9]}" for s in rep["steps"] if s["kind"] != "glue") def main(argv=None): ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) - ap.add_argument("command", choices=("sync", "verify", "report", "comment")) + ap.add_argument("command", choices=("sync", "verify", "report")) ap.add_argument("--path", required=True) ap.add_argument("--tip") ap.add_argument("--branch") @@ -411,11 +366,6 @@ def main(argv=None): ap.add_argument("--legacy", action="store_true") ap.add_argument("--json", action="store_true") ap.add_argument("--summary", action="store_true") - ap.add_argument("--image", default="") - ap.add_argument("--run-url", default="") - ap.add_argument("--branch-repo", default="") - ap.add_argument("--retire", action="store_true") - ap.add_argument("--dry-run", action="store_true") args = ap.parse_args(argv) path = Path(args.path) url_for = https_url if args.https else ssh_url @@ -453,9 +403,6 @@ def main(argv=None): rep = report(path, args.tip) print(summary(rep) if args.summary else json.dumps(rep, indent=2) if args.json else __import__("yaml").safe_dump(rep, sort_keys=False, width=140)) return 0 - for name, what in comment(path, args.tip, args.branch_repo or "?", args.branch or args.tip, args.image, args.run_url, args.retire, - dry_run=args.dry_run): - print(f"{name}: {what}") return 0 diff --git a/actions/integration/tests/test_integration.py b/actions/integration/tests/test_integration.py index f2906cf..6707da0 100644 --- a/actions/integration/tests/test_integration.py +++ b/actions/integration/tests/test_integration.py @@ -1,6 +1,5 @@ #!/usr/bin/env python3 """actions/integration/integration.py against real git repositories (the "remotes" are local repos with refs/pull/N/head).""" -import json import os import subprocess import sys @@ -218,53 +217,6 @@ def clone_run(self, *a): subprocess.run(["git", "-C", str(self.clone), *a], check=True, env=ENV, capture_output=True) -class Comments(Case): - def fake_gh(self, existing): - calls = [] - - def run(cmd, **kw): - calls.append(cmd) - if "comments" in cmd[-2:] or (len(cmd) > 2 and cmd[2].endswith("/comments")) or "--paginate" in cmd: - return subprocess.CompletedProcess(cmd, 0, stdout=json.dumps(existing), stderr="") - if any("/pulls/" in c for c in cmd): - return subprocess.CompletedProcess(cmd, 0, stdout=json.dumps({"state": "open", "title": "t", "draft": False, - "head": {"sha": "a" * 40}, "labels": []}), stderr="") - return subprocess.CompletedProcess(cmd, 0, stdout="{}", stderr="") - return run, calls - - def test_one_marker_comment_per_pr_is_created_then_edited_in_place(self): - self.remote.pr(1, "one.txt", "1\n") - self.remote.pr(2, "two.txt", "2\n") - self.sync([{"merge": "o/r#1"}, {"merge": "o/r#2"}]) - marker = integration.MARKER.format(repo="o/b", branch="integration/x") - run, calls = self.fake_gh([]) - done = integration.comment(self.clone, "integration/x", "o/b", "integration/x", "ghcr.io/o/i@sha256:abc", "https://run", run=run) - self.assertEqual([d[1] for d in done], ["created", "created"]) - posts = [c for c in calls if "POST" in c] - self.assertEqual(len(posts), 2) - body = next(a for a in posts[0] if a.startswith("body=")) - self.assertIn(marker, body) - self.assertIn("ghcr.io/o/i@sha256:abc", body) - self.assertIn("together with: o/r#2", body) - run2, calls2 = self.fake_gh([{"id": 9, "body": marker + " old"}]) - done2 = integration.comment(self.clone, "integration/x", "o/b", "integration/x", "ghcr.io/o/i@sha256:def", "", run=run2) - self.assertEqual([d[1] for d in done2], ["updated", "updated"]) - self.assertTrue(any("PATCH" in c and "issues/comments/9" in " ".join(c) for c in calls2)) - - def test_an_ephemeral_plan_posts_nothing(self): - self.remote.pr(1, "one.txt", "1\n") - self.sync([{"merge": "o/r#1"}], ephemeral=True) - run, calls = self.fake_gh([]) - self.assertEqual(integration.comment(self.clone, "integration/x", "o/b", "integration/x", "img", "", run=run), [("ephemeral", None)]) - self.assertFalse([c for c in calls if "POST" in c or "PATCH" in c]) - - def test_retire_marks_the_comment(self): - self.remote.pr(1, "one.txt", "1\n") - self.sync([{"merge": "o/r#1"}]) - run, calls = self.fake_gh([]) - integration.comment(self.clone, "integration/x", "o/b", "integration/x", "", "", retire=True, run=run) - body = next(a for c in calls if "POST" in c for a in c if a.startswith("body=")) - self.assertIn("RETIRED", body) if __name__ == "__main__": diff --git a/docs/integration-branches.md b/docs/integration-branches.md index f0037a7..b0e5072 100644 --- a/docs/integration-branches.md +++ b/docs/integration-branches.md @@ -13,7 +13,7 @@ registry. `actions/integration/integration.py` creates, verifies and reports on ```yaml version: 1 base: {repo: SUNET/vc, ref: main} -ephemeral: false # true: an ad-hoc test; no comments on the PRs +ephemeral: false # true: an ad-hoc test (reserved for the comments follow-up) steps: - merge: SUNET/vc#839 # merge the PR head (--no-ff); owner/repo#N[@sha] (the sha pins the head) - glue: {branch: sirosfoundation/vc:glue/839-831, note: resolves the seed_recover conflict} @@ -35,7 +35,7 @@ the conflicting PRs and commits the resolution), and the plan names it in a `glu `verify` classifies every glue branch: - **resolution-only**: every added line already exists in the tip before it, the base or one of the plan's PR heads. It only - chooses between existing code, so it needs no review and no PR. It is listed in the PR comments as "no new lines". + chooses between existing code, so it needs no review and no PR. It is reported as "no new lines". - **new code**: it adds lines found nowhere else. It is a change like any other: give it a draft PR and have it reviewed. `verify` prints `glue : new code (N new lines in )`. A plan marked `ephemeral: true` may skip the PR. @@ -52,20 +52,15 @@ python3 actions/integration/integration.py report --path --tip integrati Add the workflow template **Integration branch (bag of PRs)** to a repository (Actions -> New workflow). On every push to `integration/**` it: 1. runs `verify` (a branch that fails is not built); -2. builds and pushes `ghcr.io/:-` via `docker-build-push.yml` (Buildx, cache, Trivy); -3. posts or updates **one comment per participating PR**, marked `` and edited in place: the - branch, the image digest, the other PRs it was combined with, the glue branches, the run URL. The links create backlinks on the - PRs both ways. Nothing is posted for an `ephemeral` plan. +2. builds and pushes `ghcr.io/:-` via `docker-build-push.yml` (Buildx, cache, Trivy); the digest is in the run output. -Commenting on a PR in another repository (an upstream) needs the `INTEGRATION_COMMENT_TOKEN` secret: a token of a bot account (or app) -that may comment there. Without it the comment step uses the workflow's own `GITHUB_TOKEN`, which can comment only in this repository. +Cross-reference comments on the participating PRs are a planned follow-up; this version does not post any. ## Rules for consumers These images contain UNMERGED code. Never use `latest`; pin by **digest** (the build summary prints it); re-pin to a release when the PRs land. `report --json` gives each PR's state and whether its head moved since the build (the branch is then stale). ## Limits (v1) -- The comments are not retired when the branch is deleted (the plan is gone with it); `comment --retire` works while it exists. - A rebase step needs its locked head to still be fetchable (a force-pushed-away PR head cannot be replayed); merge and glue steps always can, because their heads are in the branch's own history. - Fetching other repositories' PR heads is over https: public repositories only. diff --git a/workflow-templates/integration.yml b/workflow-templates/integration.yml index 32bb7df..a2137d1 100644 --- a/workflow-templates/integration.yml +++ b/workflow-templates/integration.yml @@ -1,5 +1,5 @@ # Builds an integration branch ("bag of PRs") when integration/** is pushed - and only if it verifies against its plan -# (.integration/plan.yaml). Publishes ghcr.io/:- and comments on the participating PRs. +# (.integration/plan.yaml). Publishes ghcr.io/:-. # See https://github.com/sirosfoundation/.github/blob/main/docs/integration-branches.md name: Integration @@ -16,5 +16,3 @@ jobs: security-events: write with: image-name: sirosfoundation/integration/${{ github.event.repository.name }} - secrets: - comment-token: ${{ secrets.INTEGRATION_COMMENT_TOKEN }}