Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions .github/workflows/ext-link.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# ---------------------------------------------------------------------------
# ext-link — per-PR LINK check for the `perry-ext-*` crates (#7656)
#
# A `perry-runtime` change broke the link of five `perry-ext-*` crates and no
# per-PR gate could have caught it (#7650, fixed in #7655); it surfaced at the
# next tag, days later.
#
# Why `cargo-test`'s scope cannot see this: `ci_test_scope.py` selects a
# reverse-dependency closure, and `_is_fanout_leaf` deliberately keeps
# `perry-ext-*` / `perry-stdlib` OUT of the fan-out. That is correct on its own
# terms — their unit tests are self-contained pure-Rust logic, and re-running
# ~40 crates on every foundational change is the cost the scoping exists to
# avoid. What it misses is that for these crates the coupling is the LINK, not
# the test: they pull in a feature-stripped runtime through `perry-ffi`'s
# `runtime-link` built with `-Wl,-dead_strip`, so a new reference edge inside
# `perry-runtime` can keep alive a chain the stripper had been removing. In
# #7650 that edge was one added call (`pin_object` -> `arena::classify_heap_space`)
# in code that had previously done a raw flag write, and the symptom was
# `Undefined symbols for architecture arm64`.
#
# So this job BUILDS and does not RUN: `cargo test --no-run` links the test
# binaries and stops. Running them would add time and check nothing this does
# not already.
#
# `--release` deliberately: `-dead_strip` is what makes the failure, and it is
# a release-profile behaviour. A dev-profile build links a different set and
# would be green through exactly the regression this exists to catch.
# ---------------------------------------------------------------------------
name: ext-link

on:
pull_request:
workflow_dispatch:

concurrency:
# PR runs supersede each other; a manual dispatch is never cancelled.
group: ext-link-${{ github.event.pull_request.number || github.run_id }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

permissions:
contents: read

jobs:
ext-link:
runs-on: ubuntu-latest
# Measured on an arm64 dev Mac. COLD release target, the five crates from
# #7650 only: 7:24, 7 test binaries. All 38 ext crates with perry-runtime
# already built: 4:10, 216 test binaries. The shared `perry-runtime` release
# build dominates, so covering every ext crate costs LESS than building the
# runtime once — which is why the scope is all of them rather than the five
# that happened to fail in #7650.
#
# The bound is a backstop, not a budget: it has to cover a cold sccache
# building perry-runtime in release from scratch on a shared runner, which
# is far slower than the numbers above, while still cutting a true hang.
timeout-minutes: 120
env:
RUSTC_WRAPPER: sccache
SCCACHE_GHA_ENABLED: "false"
SCCACHE_DIR: ${{ github.workspace }}/.sccache
SCCACHE_CACHE_SIZE: "8G"
CARGO_INCREMENTAL: "0"
steps:
# This job compiles PR-controlled build scripts; don't leave the workflow
# token in .git/config for them to read.
- uses: actions/checkout@v7
with:
persist-credentials: false

# Cheap gate: no toolchain, no cargo, no cache restore. Every step below
# is skipped when the diff cannot change what the archives link.
- name: Compute ext-link scope
id: scope
env:
GH_TOKEN: ${{ github.token }}
run: |
python3 scripts/ci_ext_link_scope.py --self-test
if [ "${{ github.event_name }}" = "pull_request" ]; then
changed_files="$(gh pr view "${{ github.event.pull_request.number }}" \
--json files --jq '.files[].path')"
else
# Manual dispatch: check everything.
changed_files="crates/perry-runtime/src/lib.rs"
fi
pkgs="$(printf '%s\n' "$changed_files" | python3 scripts/ci_ext_link_scope.py)"
if [ -z "$pkgs" ]; then
echo "Diff cannot change what the ext archives link — nothing to do."
echo "pkgs=" >> "$GITHUB_OUTPUT"
else
echo "Linking $(printf '%s\n' "$pkgs" | wc -l) perry-ext-* crates:"
printf '%s\n' "$pkgs"
{
echo 'pkgs<<PERRY_EOF'
printf '%s\n' "$pkgs"
echo 'PERRY_EOF'
} >> "$GITHUB_OUTPUT"
fi

- name: Install Rust toolchain
if: steps.scope.outputs.pkgs != ''
uses: dtolnay/rust-toolchain@stable
- if: steps.scope.outputs.pkgs != ''
uses: ./.github/actions/setup-llvm22

- name: Install sccache
if: steps.scope.outputs.pkgs != ''
uses: mozilla-actions/sccache-action@v0.0.10

- name: Cache sccache objects
if: steps.scope.outputs.pkgs != ''
uses: actions/cache@v6
with:
path: ${{ github.workspace }}/.sccache
key: sccache-${{ runner.os }}-perry-${{ github.job }}-${{ github.run_id }}
restore-keys: |
sccache-${{ runner.os }}-perry-

- uses: Swatinem/rust-cache@v2
if: steps.scope.outputs.pkgs != ''
with:
shared-key: "${{ runner.os }}-perry"
save-if: ${{ github.ref == 'refs/heads/main' }}

- name: Link the ext crates
if: steps.scope.outputs.pkgs != ''
env:
# lld has repeatedly SIGBUS'd large links on the shared runner.
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS: "-C linker-features=-lld"
CARGO_PROFILE_TEST_DEBUG: "0"
CARGO_BUILD_JOBS: "1"
PKGS: ${{ steps.scope.outputs.pkgs }}
run: |
# Explicit rather than relying on the runner's default `bash -e`: a
# failing `cargo test --no-run` IS the regression this job exists to
# catch, so its exit status must abort the step under any shell.
set -euo pipefail
args=""
while read -r pkg; do
[ -n "$pkg" ] || continue
args="$args -p $pkg"
done <<< "$PKGS"

# `--message-format=json` so the count below comes from cargo rather
# than being inferred from exit status. The counting lives in the
# script (`--count-linked`) so this file carries no inline Python —
# a heredoc at column 0 silently breaks the YAML block scalar.
# shellcheck disable=SC2086
cargo test --release --no-run --message-format=json $args > /tmp/ext-link.json

linked="$(python3 scripts/ci_ext_link_scope.py --count-linked /tmp/ext-link.json)"
echo "linked test binaries: $linked"
if [ "$linked" -eq 0 ]; then
echo "::error::ext-link linked ZERO test binaries. The gate ran but had no subject — either the scope selected no packages or cargo built nothing. Fix the scope rather than trusting this green."
exit 1
fi
15 changes: 15 additions & 0 deletions changelog.d/7768-ext-link-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
### Changed

- **CI: a per-PR gate now LINKS the `perry-ext-*` crates (#7656).** A `perry-runtime` change broke the link of five of them and no per-PR gate could have caught it (#7650, fixed in #7655) — it surfaced at the next tag, days later.

`cargo-test`'s scope could not see it, and not by accident: `ci_test_scope.py`'s `_is_fanout_leaf` deliberately keeps `perry-ext-*` / `perry-stdlib` out of the reverse-dependency fan-out, because their unit tests are self-contained pure-Rust logic and re-running ~40 crates on every foundational change is the cost that scoping exists to avoid. That reasoning is right about the tests and silent about the **link**: these crates pull in a feature-stripped runtime through `perry-ffi`'s `runtime-link` built with `-Wl,-dead_strip`, so a new reference edge inside `perry-runtime` can keep alive a chain the stripper had been removing. In #7650 the edge was one added call (`pin_object` → `arena::classify_heap_space`) replacing a raw flag write, and the symptom was `Undefined symbols for architecture arm64`.

So the new `ext-link` job builds and does not run: `cargo test --release --no-run` links the test binaries and stops. `--release` deliberately — `-dead_strip` is a release-profile behaviour, and a dev-profile build would be green through exactly this regression. The existing fan-out exclusion is left alone.

**The package list is derived, not written down.** `scripts/ci_ext_link_scope.py` enumerates `crates/perry-ext-*` from the workspace, so a new ext crate is covered the day it lands — the failure mode #7748 had to repair in `ci_e2e_scope.py`, where a hand-maintained map named 3 of 24 suites and nothing could say so. Its self-test additionally asserts the five crates that actually failed in #7650 are still in the derived list, so a rename reports itself instead of quietly shrinking the gate.

**The job asserts it linked something.** #7656's first requirement, and the failure mode this repo has shipped four times (#6942/#6946, #7024, #7025): a scope rule that selected zero packages would be green forever. The count comes from cargo's own `--message-format=json` artifact records, and zero is a hard failure with an error that says to fix the scope rather than trust the green.

**Cost, measured, and why the scope is all 38 crates rather than #7650's five.** On an arm64 dev Mac: a COLD release target building only the five took **7:24** (7 test binaries); all **38** ext crates with `perry-runtime` already built took **4:10** (216 test binaries). The shared runtime build dominates, so widening the scope to every ext crate costs less than building the runtime once — the issue's "a wider set is better if it is affordable", answered with a number instead of an argument.

Not made a required context yet, per CLAUDE.md's corollary — a new gate has never been green, so promoting it immediately blocks every open PR. Run it once, then promote.
194 changes: 194 additions & 0 deletions scripts/ci_ext_link_scope.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
#!/usr/bin/env python3
"""Decide whether the per-PR `ext-link` gate must run, and over which packages.

#7656: a `perry-runtime` change broke the LINK of five `perry-ext-*` crates and
no per-PR gate could have caught it (#7650, fixed in #7655). It would have
surfaced at the next tag, days later.

## Why the existing scope cannot see this

`ci_test_scope.py` selects a reverse-dependency closure, and `_is_fanout_leaf`
deliberately keeps `perry-ext-*` / `perry-stdlib` OUT of the fan-out. That
exclusion is correct on its own terms — those crates' unit tests are
self-contained pure-Rust logic and re-running ~40 of them on every foundational
change is exactly the cost the scoping exists to avoid.

What it misses is that for these crates the coupling is not the test, it is the
**link**. They pull in a feature-stripped runtime through `perry-ffi`'s
`runtime-link`, built with `-Wl,-dead_strip`, so any new reference edge inside
`perry-runtime` can keep a chain alive the stripper had been removing. In #7650
the edge was a single added call (`pin_object` -> `arena::classify_heap_space`)
in code that had previously done a raw flag write. The symptom is
`Undefined symbols for architecture arm64`, not a failing assertion — so
BUILDING the ext crates is the entire check, and running their tests adds
nothing.

Hence a separate, link-only arm rather than widening the closure: `cargo test
--no-run` builds (and therefore links) the test binaries and stops.

## Selection

The gate runs when the diff touches a crate whose object code ends up inside
those archives — `perry-runtime`, `perry-stdlib`, `perry-ffi` — or any
`perry-ext-*` crate directly.

The package list is DERIVED from the workspace, never hand-listed: a new
`crates/perry-ext-<x>/` is covered the day it lands. A hand-maintained list is
the failure mode #7748 had to repair in `ci_e2e_scope.py`, where the map named
3 of 24 suites and nothing could say so.

Usage: <changed-files> | python3 scripts/ci_ext_link_scope.py
python3 scripts/ci_ext_link_scope.py --count-linked <cargo-json>
python3 scripts/ci_ext_link_scope.py --self-test
"""
import os
import sys

# A change to any of these can alter what the ext archives' dead-strip pass
# keeps, so they arm the gate. `perry-ffi` is here because it is the crate that
# actually declares `runtime-link`.
LINK_SOURCE_PREFIXES = (
"crates/perry-runtime/",
"crates/perry-stdlib/",
"crates/perry-ffi/",
)

EXT_PREFIX = "crates/perry-ext-"


def _repo_root() -> str:
return os.path.dirname(os.path.dirname(os.path.abspath(__file__)))


def ext_packages(root: str):
"""Every `perry-ext-*` workspace member, from the directory listing.

Derived rather than listed so a new ext crate is covered on the day it
lands. Only directories carrying a `Cargo.toml` count, so a stray path
cannot inject a bogus `-p` argument.
"""
crates = os.path.join(root, "crates")
try:
entries = os.listdir(crates)
except OSError:
return []
return sorted(
e
for e in entries
if e.startswith("perry-ext-")
and os.path.isfile(os.path.join(crates, e, "Cargo.toml"))
)


def arms(changed) -> bool:
"""Does this diff require the ext link check?"""
for path in changed:
path = path.strip()
if not path:
continue
if path.startswith(LINK_SOURCE_PREFIXES) or path.startswith(EXT_PREFIX):
return True
return False


def _self_test() -> int:
root = _repo_root()

cases = [
(["crates/perry-runtime/src/gc/mod.rs"], True),
(["crates/perry-stdlib/src/lib.rs"], True),
(["crates/perry-ffi/src/async_runtime.rs"], True),
(["crates/perry-ext-http/src/lib.rs"], True),
# Nothing that ends up in the archives.
(["crates/perry-hir/src/lower.rs"], False),
(["crates/perry-codegen/src/expr/mod.rs"], False),
(["docs/src/index.md", "CLAUDE.md"], False),
([], False),
# One armed path among many is enough.
(["README.md", "crates/perry-runtime/src/value.rs"], True),
]
for changed, expected in cases:
got = arms(changed)
if got != expected:
print(f"self-test FAILED: {changed} -> {got}, want {expected}", file=sys.stderr)
return 1

# The package list must be non-empty and must actually name ext crates. A
# scope rule that selected zero packages would make the gate green forever
# — the failure mode this repo has shipped four times (#6942/#6946, #7024,
# #7025) and the first thing #7656 asks to get right.
pkgs = ext_packages(root)
if len(pkgs) < 5:
print(
f"self-test FAILED: expected the workspace to have several "
f"perry-ext-* crates, found {len(pkgs)}: {pkgs}",
file=sys.stderr,
)
return 1
if any(not p.startswith("perry-ext-") for p in pkgs):
print(f"self-test FAILED: non-ext package in list: {pkgs}", file=sys.stderr)
return 1
# The five that actually failed in #7650 must be covered by the derived
# list — if a rename drops one, this says so instead of quietly shrinking.
for name in (
"perry-ext-pdf",
"perry-ext-lru-cache",
"perry-ext-node-forge",
"perry-ext-mongodb",
"perry-ext-http",
):
if name not in pkgs:
print(
f"self-test FAILED: {name} (one of #7650's five) is not in the "
f"derived package list",
file=sys.stderr,
)
return 1

print(f"ci_ext_link_scope self-test: ok ({len(pkgs)} ext crates)")
return 0


def count_linked(path: str) -> int:
"""Test binaries cargo reports as linked, from `--message-format=json`.

THE COUNT IS THE POINT. A scope rule that silently selected zero packages,
or a cargo invocation that built nothing, leaves the gate green forever —
the failure mode this repo has shipped four times (#6942/#6946, #7024,
#7025), and the first thing #7656 asks to get right. The job asserts this is
non-zero, so "nothing threw" is never mistaken for "the subject was live".
"""
import json

linked = 0
with open(path, encoding="utf-8", errors="replace") as fh:
for line in fh:
try:
msg = json.loads(line)
except Exception:
continue
if msg.get("reason") == "compiler-artifact" and msg.get("executable"):
linked += 1
print(f"linked {msg['target']['name']}", file=sys.stderr)
return linked


def main() -> int:
if "--self-test" in sys.argv:
return _self_test()

if "--count-linked" in sys.argv:
path = sys.argv[sys.argv.index("--count-linked") + 1]
print(count_linked(path))
return 0

changed = [line for line in sys.stdin]
if not arms(changed):
return 0
for pkg in ext_packages(_repo_root()):
print(pkg)
return 0


if __name__ == "__main__":
raise SystemExit(main())
Loading