From c86cbfb2e7c4ebfe11691a77b4b8abcce3f12015 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:18:15 +0000 Subject: [PATCH 1/2] fork-ci: one cargo invocation per step; Cargo.lock selects only affected packages Per-crate loops re-resolved features and rebuilt shared dependencies once per crate, which left a 30-vCPU runner mostly idle. fmt, clippy and nextest now take every selected package in a single invocation, and the clippy JSON stream is split back per crate so the changed-file gate is unchanged. A Cargo.lock-only change no longer rebuilds the whole workspace: the resolve graphs of the base and HEAD locks are compared and only members whose reachable dependency set moved are selected. --- .github/scripts/fork-changed-packages.py | 135 ++++++++++++++-- .github/scripts/fork-lock-affected.py | 65 ++++++++ .github/scripts/test_fork_changed_packages.py | 88 +++++++++++ .github/workflows/fork-ci.yml | 145 ++++++++++++++++-- 4 files changed, 402 insertions(+), 31 deletions(-) create mode 100644 .github/scripts/fork-lock-affected.py create mode 100644 .github/scripts/test_fork_changed_packages.py diff --git a/.github/scripts/fork-changed-packages.py b/.github/scripts/fork-changed-packages.py index b7cdcf082705..3da88702e1e0 100644 --- a/.github/scripts/fork-changed-packages.py +++ b/.github/scripts/fork-changed-packages.py @@ -4,29 +4,132 @@ one per line, on argv. A file belongs to the package whose manifest directory is the longest prefix of its path, so a nested crate wins over its parent and the directory name never has to equal the package name. + +A change to a member's Cargo.toml also selects the workspace packages that +depend on it, directly or transitively (reverse dependencies). A change to the +workspace Cargo.toml selects every package. Cargo.lock is not handled here: +the caller compares the resolve graph and passes the affected members as +ordinary paths, or prints them itself. """ import json import os import sys -meta = json.load(sys.stdin) -root = os.getcwd() -dirs = [] -for package in meta["packages"]: - manifest = os.path.relpath(package["manifest_path"], root) - dirs.append((os.path.dirname(manifest), package["name"])) -dirs.sort(key=lambda entry: len(entry[0]), reverse=True) - -seen = [] -for argument in sys.argv[1:]: - path = os.path.normpath(argument) + +def strip_workspace_prefix(path): + path = os.path.normpath(path) prefix = "codex-rs" + os.sep + marker = os.sep + "codex-rs" + os.sep + if path == "codex-rs" or path.endswith(os.sep + "codex-rs"): + return "" + if marker in path: + path = path.split(marker, 1)[1] if path.startswith(prefix): - path = path[len(prefix):] + return path[len(prefix):] + return path + + +def package_dirs(meta, root): + dirs = [] + for package in meta["packages"]: + manifest = package["manifest_path"] + if os.path.isabs(manifest): + manifest = os.path.relpath(manifest, root) + manifest = strip_workspace_prefix(manifest) + dirs.append((os.path.dirname(manifest), package["name"])) + dirs.sort(key=lambda entry: len(entry[0]), reverse=True) + return dirs + + +def owner_of(path, dirs): + path = strip_workspace_prefix(path) for directory, name in dirs: if path == directory or path.startswith(directory + os.sep): - if name not in seen: - seen.append(name) - break -print("\n".join(seen)) + return name + return None + + +def reverse_closure(meta, names): + """Workspace members that depend on `names`, directly or transitively.""" + members = set(meta.get("workspace_members", [])) + by_id = {package["id"]: package for package in meta["packages"]} + # name -> packages in the workspace that list it as a normal/dev/build dep + dependents = {} + for package in meta["packages"]: + if package["id"] not in members: + continue + for dep in package.get("dependencies", []): + dep_id = dep.get("pkg") or dep.get("id") + target = by_id.get(dep_id) + if target is None: + continue + dependents.setdefault(target["name"], set()).add(package["name"]) + selected = set(names) + frontier = list(names) + while frontier: + name = frontier.pop() + for dependent in dependents.get(name, ()): + if dependent not in selected: + selected.add(dependent) + frontier.append(dependent) + return selected + + +def select(meta, paths, root=None): + """Return workspace package names selected by `paths`, in first-seen order. + + `paths` are repo-relative. The workspace Cargo.toml selects everything. + A member Cargo.toml selects that member and its reverse dependencies. + Any other file selects only its owning package. + """ + root = root or os.getcwd() + dirs = package_dirs(meta, root) + member_names = [] + for package in meta["packages"]: + if package["id"] in set(meta.get("workspace_members", [])): + member_names.append(package["name"]) + manifest_dir = {} + for package in meta["packages"]: + manifest = package["manifest_path"] + if os.path.isabs(manifest): + manifest = os.path.relpath(manifest, root) + manifest = strip_workspace_prefix(manifest) + manifest_dir[package["name"]] = os.path.dirname(manifest) + + seen = [] + + def add(name): + if name and name not in seen: + seen.append(name) + + manifest_hits = [] + for argument in paths: + path = os.path.normpath(argument) + stripped = strip_workspace_prefix(path) + if stripped in ("Cargo.toml", "./Cargo.toml"): + return member_names + owner = owner_of(path, dirs) + if owner is None: + continue + if os.path.basename(stripped) == "Cargo.toml" and manifest_dir.get(owner) == os.path.dirname(stripped): + manifest_hits.append(owner) + else: + add(owner) + if manifest_hits: + closure = reverse_closure(meta, manifest_hits) + for name in member_names: + if name in closure: + add(name) + for name in manifest_hits: + add(name) + return seen + + +def main(argv): + meta = json.load(sys.stdin) + print("\n".join(select(meta, argv[1:]))) + + +if __name__ == "__main__": + main(sys.argv) diff --git a/.github/scripts/fork-lock-affected.py b/.github/scripts/fork-lock-affected.py new file mode 100644 index 000000000000..f71b7a005d8f --- /dev/null +++ b/.github/scripts/fork-lock-affected.py @@ -0,0 +1,65 @@ +"""Workspace packages whose resolved dependency set changed in Cargo.lock. + +Compares two `cargo metadata --format-version 1` documents (the full resolve +graph, not `--no-deps`). A workspace member is affected when the set of +package ids reachable from it through normal, dev and build dependencies +differs between the two resolves. Prints one package name per line. + +Usage: fork-lock-affected.py BASE.json HEAD.json +""" + +import json +import sys + + +def reachable(meta): + """name -> frozenset of resolved package ids reachable from that member.""" + by_id = {package["id"]: package for package in meta["packages"]} + members = [ + (package["id"], package["name"]) + for package in meta["packages"] + if package["id"] in set(meta.get("workspace_members", [])) + ] + cache = {} + + def walk(package_id, stack): + if package_id in cache: + return cache[package_id] + if package_id in stack or package_id not in by_id: + return frozenset() + stack.add(package_id) + found = {package_id} + for dep in by_id[package_id].get("dependencies", []): + dep_id = dep.get("pkg") or dep.get("id") + if dep_id: + found |= walk(dep_id, stack) + stack.remove(package_id) + cache[package_id] = frozenset(found) + return cache[package_id] + + return {name: walk(package_id, set()) for package_id, name in members} + + +def affected(base, head): + base_sets = reachable(base) + head_sets = reachable(head) + names = [] + for name in head_sets: + if base_sets.get(name) != head_sets[name]: + names.append(name) + for name in base_sets: + if name not in head_sets and name not in names: + names.append(name) + return names + + +def main(argv): + with open(argv[1], encoding="utf-8") as handle: + base = json.load(handle) + with open(argv[2], encoding="utf-8") as handle: + head = json.load(handle) + print("\n".join(affected(base, head))) + + +if __name__ == "__main__": + main(sys.argv) diff --git a/.github/scripts/test_fork_changed_packages.py b/.github/scripts/test_fork_changed_packages.py new file mode 100644 index 000000000000..7cb1df7029ff --- /dev/null +++ b/.github/scripts/test_fork_changed_packages.py @@ -0,0 +1,88 @@ +"""Selection semantics for fork-ci's changed-package scripts.""" + +import json +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +import importlib.util + + +def load(name, filename): + spec = importlib.util.spec_from_file_location( + name, os.path.join(os.path.dirname(os.path.abspath(__file__)), filename) + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +fork_changed_packages = load("fork_changed_packages", "fork-changed-packages.py") +fork_lock_affected = load("fork_lock_affected", "fork-lock-affected.py") + + +def package(name, directory, deps): + return { + "name": name, + "id": f"{directory}-id", + "manifest_path": os.path.join("/work/codex-rs", directory, "Cargo.toml"), + "dependencies": [{"pkg": dep} for dep in deps], + } + + +META = { + "workspace_members": ["core-id", "cli-id", "protocol-id"], + "packages": [ + package("codex-core", "core", ["protocol-id"]), + package("codex-cli", "cli", ["core-id"]), + package("codex-protocol", "protocol", []), + ], +} + + +class SelectTest(unittest.TestCase): + def select(self, paths): + return fork_changed_packages.select(META, paths, root="/work/codex-rs") + + def test_source_file_selects_its_package(self): + self.assertEqual(self.select(["codex-rs/core/src/lib.rs"]), ["codex-core"]) + + def test_workspace_manifest_selects_everything(self): + self.assertEqual( + self.select(["codex-rs/Cargo.toml"]), + ["codex-core", "codex-cli", "codex-protocol"], + ) + + def test_member_manifest_selects_reverse_dependencies(self): + selected = self.select(["codex-rs/protocol/Cargo.toml"]) + self.assertEqual(selected, ["codex-core", "codex-cli", "codex-protocol"]) + + def test_member_manifest_without_dependents_selects_itself(self): + self.assertEqual(self.select(["codex-rs/cli/Cargo.toml"]), ["codex-cli"]) + + +class LockTest(unittest.TestCase): + def test_lock_only_change_selects_the_affected_subset(self): + base = { + "workspace_members": ["core-id", "cli-id", "proto-id"], + "packages": [ + {"name": "serde", "id": "serde-1", "dependencies": []}, + {"name": "codex-protocol", "id": "proto-id", "dependencies": []}, + {"name": "codex-core", "id": "core-id", "dependencies": [{"pkg": "serde-1"}, {"pkg": "proto-id"}]}, + {"name": "codex-cli", "id": "cli-id", "dependencies": [{"pkg": "core-id"}]}, + ], + } + head = json.loads(json.dumps(base)) + for package in head["packages"]: + if package["id"] == "serde-1": + package["id"] = "serde-2" + for dep in package["dependencies"]: + if dep["pkg"] == "serde-1": + dep["pkg"] = "serde-2" + self.assertEqual(fork_lock_affected.affected(base, head), ["codex-core", "codex-cli"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 7e0dac9d6428..1b35ff40f338 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -30,6 +30,20 @@ jobs: defaults: run: working-directory: codex-rs + env: + # CI-scoped build settings from the cargo build-performance guide. + # Job env only: upstream's profiles in Cargo.toml stay untouched. + # line-tables-only keeps file/line in backtraces (tests do read them) + # while dropping variable debuginfo, which dominates codegen and linking. + CARGO_PROFILE_DEV_DEBUG: line-tables-only + CARGO_PROFILE_TEST_DEBUG: line-tables-only + # CI builds once per crate version; incremental caches never pay back. + CARGO_INCREMENTAL: "0" + # No RUSTFLAGS. Stable rustc 1.95 on x86_64-unknown-linux-gnu already + # links with its bundled rust-lld (the self-contained linker, shipped in + # the sysroot since 1.90). -fuse-ld=lld would need a system lld and a + # different linker, and nightly-only options (-Zthreads, cranelift) are + # out: this job pins stable. steps: - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.0.1 with: @@ -108,18 +122,51 @@ jobs: exit 0 fi all=false + lock_only=false for f in "${files[@]}"; do case "$f" in - codex-rs/Cargo.toml|codex-rs/Cargo.lock) all=true ;; + codex-rs/Cargo.toml) all=true ;; + codex-rs/Cargo.lock) lock_only=true ;; esac done + # A workspace manifest change rebuilds every package. A Cargo.lock + # change does not: only members whose resolved dependency set + # actually moved are selected, by diffing `cargo metadata` (the full + # resolve, not --no-deps) on the base against HEAD. Any other path + # still selects its owning package, and a member manifest also + # selects its workspace reverse-dependencies. + others=() + for f in "${files[@]}"; do + [[ "$f" == "codex-rs/Cargo.lock" ]] && continue + others+=("$f") + done if $all; then mapfile -t packages < <(cargo metadata --no-deps --format-version 1 \ | jq -r '.packages[].name' | sort -u) else mapfile -t packages < <(cargo metadata --no-deps --format-version 1 \ - | python3 "${GITHUB_WORKSPACE}/.github/scripts/fork-changed-packages.py" "${files[@]}") + | python3 "${GITHUB_WORKSPACE}/.github/scripts/fork-changed-packages.py" "${others[@]}") + if $lock_only; then + base_meta="$(mktemp)" + head_meta="$(mktemp)" + cargo metadata --format-version 1 --locked > "$head_meta" + # metadata reads the lock next to the workspace manifest. Swap the + # base lock in, read it, and put HEAD's lock back. The manifest + # stays HEAD's: a manifest change is not lock-only. + cp Cargo.lock /tmp/Cargo.lock.head + # A failed metadata read must not leave the base lock checked out: + # every later cargo invocation would compile against it. + trap 'mv /tmp/Cargo.lock.head Cargo.lock' EXIT + git show "$base:codex-rs/Cargo.lock" > Cargo.lock + cargo metadata --format-version 1 --locked > "$base_meta" + mv /tmp/Cargo.lock.head Cargo.lock + trap - EXIT + mapfile -t locked < <(python3 "${GITHUB_WORKSPACE}/.github/scripts/fork-lock-affected.py" "$base_meta" "$head_meta") + packages+=("${locked[@]}") + fi fi + # Unique, stable order. + mapfile -t packages < <(printf '%s\n' "${packages[@]}" | awk 'NF && !seen[$0]++') echo "changed: ${packages[*]:-none}" { echo "list< /tmp/changed-files - while IFS= read -r crate; do - [[ -z "$crate" ]] && continue - cargo clippy -p "$crate" --all-targets --message-format=json -- -W clippy::all \ - > "/tmp/clippy-${crate}.json" || true + mapfile -t crates < <(printf '%s\n' "${{ steps.crates.outputs.list }}" | sed '/^$/d') + (( ${#crates[@]} )) || exit 0 + args=() + for crate in "${crates[@]}"; do args+=(-p "$crate"); done + # One invocation for every selected package. Per-crate loops re-resolve + # features and rebuild shared dependencies once per crate; cargo shares + # that work when the packages are passed together. --timings writes + # cargo-timing.html, which the next cargo step overwrites, so copy it + # aside immediately. + mkdir -p /tmp/cargo-timings + cargo clippy "${args[@]}" --all-targets --timings --message-format=json -- -W clippy::all \ + > /tmp/clippy.json || true + [[ -f cargo-timing.html ]] && cp cargo-timing.html /tmp/cargo-timings/clippy-timing.html + # The single JSON stream mixes every package. Split it back into one + # file per crate on package_id (falling back to the primary span's + # path) so the per-crate report below is unchanged. + printf '%s\n' "${crates[@]}" > /tmp/crates + python3 - <<'PY' + import json + crates = [line.strip() for line in open("/tmp/crates") if line.strip()] + # Longest name first, so a prefix of another package name cannot win. + by_length = sorted(crates, key=len, reverse=True) + buckets = {crate: [] for crate in crates} + other = [] + + def attribute(package_id): + # cargo's package_id is `name version (source)`, a path+url spec, + # or `registry+url#name@version`. The leading token is only the + # name in the first of those. + token = package_id.split()[0] if package_id else "" + if token in buckets: + return token + for crate in by_length: + if (f"#{crate}@" in package_id) or package_id.startswith(crate + " "): + return crate + return "" + + for line in open("/tmp/clippy.json"): + line = line.strip() + if not line: + continue + try: + msg = json.loads(line) + except json.JSONDecodeError: + other.append(line) + continue + name = attribute(msg.get("package_id", "")) + (buckets[name] if name in buckets else other).append(line) + for crate, lines in buckets.items(): + with open(f"/tmp/clippy-{crate}.json", "w") as handle: + handle.write("\n".join(lines) + ("\n" if lines else "")) + if other: + with open("/tmp/clippy-unattributed.json", "w") as handle: + handle.write("\n".join(other) + "\n") + PY + for crate in "${crates[@]}"; do # --slurpfile yields an array of strings. Iterating each element # again (`[$changed[][]]`) is a jq runtime error, and jq exits 0 on # those, so the gate would pass every PR. `any` compares strings @@ -195,7 +295,7 @@ jobs: echo "clippy findings in files this PR changes" >&2 exit 1 fi - done <<< "${{ steps.crates.outputs.list }}" + done - name: cargo test shell: bash @@ -235,8 +335,23 @@ jobs: # tests at its first failure, so --no-fail-fast does the same within # a crate. status=0 - while IFS= read -r crate; do - [[ -z "$crate" ]] && continue - cargo nextest run -p "$crate" --no-fail-fast "${filter[@]}" || status=1 - done <<< "${{ steps.crates.outputs.list }}" + mapfile -t crates < <(printf '%s\n' "${{ steps.crates.outputs.list }}" | sed '/^$/d') + if (( ${#crates[@]} )); then + args=() + for crate in "${crates[@]}"; do args+=(-p "$crate"); done + # One invocation: per-crate loops re-resolved features and rebuilt + # shared dependencies once per crate. --timings records the + # critical path next to clippy's. + cargo nextest run "${args[@]}" --no-fail-fast --timings "${filter[@]}" || status=1 + [[ -f cargo-timing.html ]] && cp cargo-timing.html /tmp/cargo-timings/nextest-timing.html + fi exit "$status" + + # Uploaded even when the job failed: the report is most useful then. + - name: Upload cargo timings + if: always() + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 + with: + name: cargo-timings + path: /tmp/cargo-timings/ + if-no-files-found: ignore From e7817b0350b27d4a22595299b8e352a371b7d7b4 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 10 Oct 2026 13:37:38 +0000 Subject: [PATCH 2/2] fork-ci: select real lock edges and gate every clippy line The lock diff walked packages[].dependencies, which cargo metadata never puts package ids on, so a lock-only bump selected nothing. Walk resolve.nodes[].deps instead, and match declared path deps by name for the --no-deps reverse closure. If a Cargo.lock change still selects nothing, select every member rather than skip the checks. The clippy gate split the JSON per crate and never read the unattributed bucket, which is where every lint from a crate whose package id omits its name (codex-home and five others) landed. Gate the whole stream once, by changed file path. --- .github/scripts/fork-changed-packages.py | 22 +- .github/scripts/fork-lock-affected.py | 29 +- .github/scripts/test_fork_changed_packages.py | 115 +++++-- .github/scripts/testdata/lock-base.json | 315 ++++++++++++++++++ .github/scripts/testdata/lock-head.json | 315 ++++++++++++++++++ .github/scripts/testdata/nodeps.json | 136 ++++++++ .github/workflows/fork-ci.yml | 101 ++---- 7 files changed, 915 insertions(+), 118 deletions(-) create mode 100644 .github/scripts/testdata/lock-base.json create mode 100644 .github/scripts/testdata/lock-head.json create mode 100644 .github/scripts/testdata/nodeps.json diff --git a/.github/scripts/fork-changed-packages.py b/.github/scripts/fork-changed-packages.py index 3da88702e1e0..d5eed743e866 100644 --- a/.github/scripts/fork-changed-packages.py +++ b/.github/scripts/fork-changed-packages.py @@ -51,20 +51,28 @@ def owner_of(path, dirs): def reverse_closure(meta, names): - """Workspace members that depend on `names`, directly or transitively.""" + """Workspace members that depend on `names`, directly or transitively. + + The selection step runs `cargo metadata --no-deps`, whose `resolve` is null, + so the only edges available are the declared requirements. A declared + dependency names a workspace member when it has a `path` or its `name` is + itself a member; registry requirements match neither. + """ members = set(meta.get("workspace_members", [])) - by_id = {package["id"]: package for package in meta["packages"]} + member_names = { + package["name"] + for package in meta["packages"] + if package["id"] in members + } # name -> packages in the workspace that list it as a normal/dev/build dep dependents = {} for package in meta["packages"]: if package["id"] not in members: continue for dep in package.get("dependencies", []): - dep_id = dep.get("pkg") or dep.get("id") - target = by_id.get(dep_id) - if target is None: - continue - dependents.setdefault(target["name"], set()).add(package["name"]) + dep_name = dep.get("name") + if dep.get("path") or dep_name in member_names: + dependents.setdefault(dep_name, set()).add(package["name"]) selected = set(names) frontier = list(names) while frontier: diff --git a/.github/scripts/fork-lock-affected.py b/.github/scripts/fork-lock-affected.py index f71b7a005d8f..07c050923d41 100644 --- a/.github/scripts/fork-lock-affected.py +++ b/.github/scripts/fork-lock-affected.py @@ -12,9 +12,28 @@ import sys +# cargo metadata records resolved edges on resolve.nodes[].deps, not on the +# declared packages[].dependencies list (that one has name/req/path and no +# package id). None is cargo's kind for a normal dependency. +KEPT_KINDS = (None, "dev", "build") + + +def edges(meta): + """package id -> resolved package ids it depends on (normal, dev, build).""" + graph = {} + for node in (meta.get("resolve") or {}).get("nodes", []): + deps = set() + for dep in node.get("deps", []): + kinds = [entry.get("kind") for entry in dep.get("dep_kinds", [])] + if not kinds or any(kind in KEPT_KINDS for kind in kinds): + deps.add(dep["pkg"]) + graph[node["id"]] = deps + return graph + + def reachable(meta): """name -> frozenset of resolved package ids reachable from that member.""" - by_id = {package["id"]: package for package in meta["packages"]} + graph = edges(meta) members = [ (package["id"], package["name"]) for package in meta["packages"] @@ -25,14 +44,12 @@ def reachable(meta): def walk(package_id, stack): if package_id in cache: return cache[package_id] - if package_id in stack or package_id not in by_id: + if package_id in stack or package_id not in graph: return frozenset() stack.add(package_id) found = {package_id} - for dep in by_id[package_id].get("dependencies", []): - dep_id = dep.get("pkg") or dep.get("id") - if dep_id: - found |= walk(dep_id, stack) + for dep_id in graph[package_id]: + found |= walk(dep_id, stack) stack.remove(package_id) cache[package_id] = frozenset(found) return cache[package_id] diff --git a/.github/scripts/test_fork_changed_packages.py b/.github/scripts/test_fork_changed_packages.py index 7cb1df7029ff..be4773d3fbf8 100644 --- a/.github/scripts/test_fork_changed_packages.py +++ b/.github/scripts/test_fork_changed_packages.py @@ -1,8 +1,17 @@ -"""Selection semantics for fork-ci's changed-package scripts.""" +"""Selection semantics for fork-ci's changed-package scripts. + +The fixtures are real `cargo metadata --format-version 1` output from a +three-member scratch workspace (codex-protocol, codex-core, codex-cli), trimmed +to the packages the graph walk reaches. `nodeps.json` was generated with +`--no-deps` (resolve is null); `lock-base.json` and `lock-head.json` are full +resolves whose only difference is the pinned serde version. +""" import json import os +import subprocess import sys +import tempfile import unittest sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) @@ -19,32 +28,23 @@ def load(name, filename): return module +HERE = os.path.dirname(os.path.abspath(__file__)) fork_changed_packages = load("fork_changed_packages", "fork-changed-packages.py") fork_lock_affected = load("fork_lock_affected", "fork-lock-affected.py") -def package(name, directory, deps): - return { - "name": name, - "id": f"{directory}-id", - "manifest_path": os.path.join("/work/codex-rs", directory, "Cargo.toml"), - "dependencies": [{"pkg": dep} for dep in deps], - } +def fixture(name): + with open(os.path.join(HERE, "testdata", name), encoding="utf-8") as handle: + return json.load(handle) -META = { - "workspace_members": ["core-id", "cli-id", "protocol-id"], - "packages": [ - package("codex-core", "core", ["protocol-id"]), - package("codex-cli", "cli", ["core-id"]), - package("codex-protocol", "protocol", []), - ], -} +NODEPS = fixture("nodeps.json") +ROOT = NODEPS["workspace_root"] class SelectTest(unittest.TestCase): def select(self, paths): - return fork_changed_packages.select(META, paths, root="/work/codex-rs") + return fork_changed_packages.select(NODEPS, paths, root=ROOT) def test_source_file_selects_its_package(self): self.assertEqual(self.select(["codex-rs/core/src/lib.rs"]), ["codex-core"]) @@ -52,12 +52,12 @@ def test_source_file_selects_its_package(self): def test_workspace_manifest_selects_everything(self): self.assertEqual( self.select(["codex-rs/Cargo.toml"]), - ["codex-core", "codex-cli", "codex-protocol"], + ["codex-protocol", "codex-core", "codex-cli"], ) def test_member_manifest_selects_reverse_dependencies(self): selected = self.select(["codex-rs/protocol/Cargo.toml"]) - self.assertEqual(selected, ["codex-core", "codex-cli", "codex-protocol"]) + self.assertEqual(selected, ["codex-protocol", "codex-core", "codex-cli"]) def test_member_manifest_without_dependents_selects_itself(self): self.assertEqual(self.select(["codex-rs/cli/Cargo.toml"]), ["codex-cli"]) @@ -65,23 +65,68 @@ def test_member_manifest_without_dependents_selects_itself(self): class LockTest(unittest.TestCase): def test_lock_only_change_selects_the_affected_subset(self): - base = { - "workspace_members": ["core-id", "cli-id", "proto-id"], - "packages": [ - {"name": "serde", "id": "serde-1", "dependencies": []}, - {"name": "codex-protocol", "id": "proto-id", "dependencies": []}, - {"name": "codex-core", "id": "core-id", "dependencies": [{"pkg": "serde-1"}, {"pkg": "proto-id"}]}, - {"name": "codex-cli", "id": "cli-id", "dependencies": [{"pkg": "core-id"}]}, - ], + base = fixture("lock-base.json") + head = fixture("lock-head.json") + # serde 1.0.200 -> 1.0.210. codex-core depends on it directly and + # codex-cli only through codex-core, so both reachable sets move. + self.assertEqual( + fork_lock_affected.affected(base, head), ["codex-cli", "codex-core"] + ) + + def test_identical_resolves_select_nothing(self): + head = fixture("lock-head.json") + self.assertEqual(fork_lock_affected.affected(head, head), []) + + +CLIPPY_GATE = r""" +select(.reason == "compiler-message") +| .message +| select(.level == "warning" or .level == "error") +| . as $m +| ($m.spans[] | select(.is_primary)) as $s +| select(any($changed[]; . == $s.file_name)) +| "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" +""" + + +class ClippyGateTest(unittest.TestCase): + def test_nameless_package_id_in_a_changed_file_fails_the_gate(self): + # cargo omits the name from a package id when it equals the directory's + # last segment, so codex-home's id has no `#codex-home@` to match on. + line = { + "reason": "compiler-message", + "package_id": "path+file:///work/codex-rs/codex-home#0.1.0", + "message": { + "level": "warning", + "message": "this function has too many arguments", + "spans": [ + { + "file_name": "codex-home/src/lib.rs", + "line_start": 12, + "column_start": 1, + "is_primary": True, + } + ], + }, } - head = json.loads(json.dumps(base)) - for package in head["packages"]: - if package["id"] == "serde-1": - package["id"] = "serde-2" - for dep in package["dependencies"]: - if dep["pkg"] == "serde-1": - dep["pkg"] = "serde-2" - self.assertEqual(fork_lock_affected.affected(base, head), ["codex-core", "codex-cli"]) + with tempfile.TemporaryDirectory() as directory: + clippy = os.path.join(directory, "clippy.json") + changed = os.path.join(directory, "changed") + with open(clippy, "w", encoding="utf-8") as handle: + handle.write(json.dumps(line) + "\n") + with open(changed, "w", encoding="utf-8") as handle: + handle.write("codex-home/src/lib.rs\n") + # The workflow slurps `jq -R .` of the changed-file list, which + # yields an array of JSON strings. Slurping the raw file would + # yield one string with the newline still in it. + quoted = os.path.join(directory, "changed.json") + with open(quoted, "w", encoding="utf-8") as handle: + subprocess.run(["jq", "-R", ".", changed], check=True, stdout=handle) + result = subprocess.run( + ["jq", "-r", "--slurpfile", "changed", quoted, CLIPPY_GATE, clippy], + check=True, capture_output=True, text=True, + ) + self.assertIn("codex-home/src/lib.rs:12:1: warning:", result.stdout) if __name__ == "__main__": diff --git a/.github/scripts/testdata/lock-base.json b/.github/scripts/testdata/lock-base.json new file mode 100644 index 000000000000..89dcdfb1d949 --- /dev/null +++ b/.github/scripts/testdata/lock-base.json @@ -0,0 +1,315 @@ +{ + "version": 1, + "workspace_members": [ + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0" + ], + "workspace_root": "/tmp/fix-ws", + "packages": [ + { + "name": "codex-cli", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0", + "manifest_path": "/tmp/fix-ws/cli/Cargo.toml", + "dependencies": [ + { + "name": "codex-core", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/core", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "codex-protocol", + "kind": "dev", + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_cli", + "src_path": "/tmp/fix-ws/cli/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-core", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "manifest_path": "/tmp/fix-ws/core/Cargo.toml", + "dependencies": [ + { + "name": "codex-protocol", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde", + "kind": null, + "optional": false, + "req": "=1.0.200", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_core", + "src_path": "/tmp/fix-ws/core/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-protocol", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "manifest_path": "/tmp/fix-ws/protocol/Cargo.toml", + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_protocol", + "src_path": "/tmp/fix-ws/protocol/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "serde", + "version": "1.0.200", + "id": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.200", + "manifest_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.200/Cargo.toml", + "dependencies": [ + { + "name": "serde_derive", + "kind": null, + "optional": true, + "req": "^1", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde_derive", + "kind": "dev", + "optional": false, + "req": "^1", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde_derive", + "kind": null, + "optional": false, + "req": "=1.0.200", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": "cfg(any())" + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "serde", + "src_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.200/src/lib.rs", + "edition": "2018", + "doc": true, + "doctest": true, + "test": true + }, + { + "kind": [ + "custom-build" + ], + "crate_types": [ + "bin" + ], + "name": "build-script-build", + "src_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.200/build.rs", + "edition": "2018", + "doc": false, + "doctest": false, + "test": false + } + ], + "features": { + "alloc": [], + "default": [ + "std" + ], + "derive": [ + "serde_derive" + ], + "rc": [], + "serde_derive": [ + "dep:serde_derive" + ], + "std": [], + "unstable": [] + }, + "source": "registry+https://github.com/rust-lang/crates.io-index" + } + ], + "resolve": { + "nodes": [ + { + "id": "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0", + "dependencies": [ + "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0" + ], + "deps": [ + { + "name": "codex_core", + "pkg": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + }, + { + "name": "codex_protocol", + "pkg": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dep_kinds": [ + { + "kind": "dev", + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "dependencies": [ + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.200" + ], + "deps": [ + { + "name": "codex_protocol", + "pkg": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + }, + { + "name": "serde", + "pkg": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.200", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dependencies": [], + "deps": [], + "features": [] + }, + { + "id": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.200", + "dependencies": [ + "registry+https://github.com/rust-lang/crates.io-index#serde_derive@1.0.200" + ], + "deps": [ + { + "name": "serde_derive", + "pkg": "registry+https://github.com/rust-lang/crates.io-index#serde_derive@1.0.200", + "dep_kinds": [ + { + "kind": null, + "target": "cfg(any())" + } + ] + } + ], + "features": [ + "default", + "std" + ] + } + ], + "root": null + } +} diff --git a/.github/scripts/testdata/lock-head.json b/.github/scripts/testdata/lock-head.json new file mode 100644 index 000000000000..4972628172a0 --- /dev/null +++ b/.github/scripts/testdata/lock-head.json @@ -0,0 +1,315 @@ +{ + "version": 1, + "workspace_members": [ + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0" + ], + "workspace_root": "/tmp/fix-ws", + "packages": [ + { + "name": "codex-cli", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0", + "manifest_path": "/tmp/fix-ws/cli/Cargo.toml", + "dependencies": [ + { + "name": "codex-core", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/core", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "codex-protocol", + "kind": "dev", + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_cli", + "src_path": "/tmp/fix-ws/cli/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-core", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "manifest_path": "/tmp/fix-ws/core/Cargo.toml", + "dependencies": [ + { + "name": "codex-protocol", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde", + "kind": null, + "optional": false, + "req": "=1.0.210", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_core", + "src_path": "/tmp/fix-ws/core/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-protocol", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "manifest_path": "/tmp/fix-ws/protocol/Cargo.toml", + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_protocol", + "src_path": "/tmp/fix-ws/protocol/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "serde", + "version": "1.0.210", + "id": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.210", + "manifest_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.210/Cargo.toml", + "dependencies": [ + { + "name": "serde_derive", + "kind": null, + "optional": true, + "req": "^1", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde_derive", + "kind": "dev", + "optional": false, + "req": "^1", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde_derive", + "kind": null, + "optional": false, + "req": "=1.0.210", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": "cfg(any())" + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "serde", + "src_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.210/src/lib.rs", + "edition": "2018", + "doc": true, + "doctest": true, + "test": true + }, + { + "kind": [ + "custom-build" + ], + "crate_types": [ + "bin" + ], + "name": "build-script-build", + "src_path": "/opt/rust/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/serde-1.0.210/build.rs", + "edition": "2018", + "doc": false, + "doctest": false, + "test": false + } + ], + "features": { + "alloc": [], + "default": [ + "std" + ], + "derive": [ + "serde_derive" + ], + "rc": [], + "serde_derive": [ + "dep:serde_derive" + ], + "std": [], + "unstable": [] + }, + "source": "registry+https://github.com/rust-lang/crates.io-index" + } + ], + "resolve": { + "nodes": [ + { + "id": "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0", + "dependencies": [ + "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0" + ], + "deps": [ + { + "name": "codex_core", + "pkg": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + }, + { + "name": "codex_protocol", + "pkg": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dep_kinds": [ + { + "kind": "dev", + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "dependencies": [ + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.210" + ], + "deps": [ + { + "name": "codex_protocol", + "pkg": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + }, + { + "name": "serde", + "pkg": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.210", + "dep_kinds": [ + { + "kind": null, + "target": null + } + ] + } + ], + "features": [] + }, + { + "id": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "dependencies": [], + "deps": [], + "features": [] + }, + { + "id": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.210", + "dependencies": [ + "registry+https://github.com/rust-lang/crates.io-index#serde_derive@1.0.210" + ], + "deps": [ + { + "name": "serde_derive", + "pkg": "registry+https://github.com/rust-lang/crates.io-index#serde_derive@1.0.210", + "dep_kinds": [ + { + "kind": null, + "target": "cfg(any())" + } + ] + } + ], + "features": [ + "default", + "std" + ] + } + ], + "root": null + } +} diff --git a/.github/scripts/testdata/nodeps.json b/.github/scripts/testdata/nodeps.json new file mode 100644 index 000000000000..fe3c7f4f3230 --- /dev/null +++ b/.github/scripts/testdata/nodeps.json @@ -0,0 +1,136 @@ +{ + "version": 1, + "workspace_members": [ + "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0" + ], + "workspace_root": "/tmp/fix-ws", + "resolve": null, + "packages": [ + { + "name": "codex-protocol", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/protocol#codex-protocol@0.1.0", + "manifest_path": "/tmp/fix-ws/protocol/Cargo.toml", + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_protocol", + "src_path": "/tmp/fix-ws/protocol/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-core", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/core#codex-core@0.1.0", + "manifest_path": "/tmp/fix-ws/core/Cargo.toml", + "dependencies": [ + { + "name": "codex-protocol", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "serde", + "kind": null, + "optional": false, + "req": "=1.0.210", + "source": "registry+https://github.com/rust-lang/crates.io-index", + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_core", + "src_path": "/tmp/fix-ws/core/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + }, + { + "name": "codex-cli", + "version": "0.1.0", + "id": "path+file:///tmp/fix-ws/cli#codex-cli@0.1.0", + "manifest_path": "/tmp/fix-ws/cli/Cargo.toml", + "dependencies": [ + { + "name": "codex-core", + "kind": null, + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/core", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + }, + { + "name": "codex-protocol", + "kind": "dev", + "optional": false, + "req": "*", + "path": "/tmp/fix-ws/protocol", + "source": null, + "rename": null, + "uses_default_features": true, + "features": [], + "target": null + } + ], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "codex_cli", + "src_path": "/tmp/fix-ws/cli/src/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "source": null + } + ] +} diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 1b35ff40f338..1a532e1da686 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -162,6 +162,13 @@ jobs: mv /tmp/Cargo.lock.head Cargo.lock trap - EXIT mapfile -t locked < <(python3 "${GITHUB_WORKSPACE}/.github/scripts/fork-lock-affected.py" "$base_meta" "$head_meta") + if (( ${#locked[@]} == 0 )); then + # A lock that "affects nothing" is a bug in the diff far more + # often than a fact, and skipping every check would fail open. + echo "::warning::Cargo.lock changed but the resolve diff selected nothing; selecting every member" + mapfile -t locked < <(jq -r --argjson members "$(jq -c '.workspace_members' "$head_meta")" \ + '.packages[] | select(.id as $id | $members | index($id)) | .name' "$head_meta" | sort -u) + fi packages+=("${locked[@]}") fi fi @@ -226,76 +233,30 @@ jobs: cargo clippy "${args[@]}" --all-targets --timings --message-format=json -- -W clippy::all \ > /tmp/clippy.json || true [[ -f cargo-timing.html ]] && cp cargo-timing.html /tmp/cargo-timings/clippy-timing.html - # The single JSON stream mixes every package. Split it back into one - # file per crate on package_id (falling back to the primary span's - # path) so the per-crate report below is unchanged. - printf '%s\n' "${crates[@]}" > /tmp/crates - python3 - <<'PY' - import json - crates = [line.strip() for line in open("/tmp/crates") if line.strip()] - # Longest name first, so a prefix of another package name cannot win. - by_length = sorted(crates, key=len, reverse=True) - buckets = {crate: [] for crate in crates} - other = [] - - def attribute(package_id): - # cargo's package_id is `name version (source)`, a path+url spec, - # or `registry+url#name@version`. The leading token is only the - # name in the first of those. - token = package_id.split()[0] if package_id else "" - if token in buckets: - return token - for crate in by_length: - if (f"#{crate}@" in package_id) or package_id.startswith(crate + " "): - return crate - return "" - - for line in open("/tmp/clippy.json"): - line = line.strip() - if not line: - continue - try: - msg = json.loads(line) - except json.JSONDecodeError: - other.append(line) - continue - name = attribute(msg.get("package_id", "")) - (buckets[name] if name in buckets else other).append(line) - for crate, lines in buckets.items(): - with open(f"/tmp/clippy-{crate}.json", "w") as handle: - handle.write("\n".join(lines) + ("\n" if lines else "")) - if other: - with open("/tmp/clippy-unattributed.json", "w") as handle: - handle.write("\n".join(other) + "\n") - PY - for crate in "${crates[@]}"; do - # --slurpfile yields an array of strings. Iterating each element - # again (`[$changed[][]]`) is a jq runtime error, and jq exits 0 on - # those, so the gate would pass every PR. `any` compares strings - # directly. stderr goes to a file synchronously: a process - # substitution can still be writing when the check runs, which - # would let a jq error pass again. - rm -f /tmp/jq-err - offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' - select(.reason == "compiler-message") - | .message - | select(.level == "warning" or .level == "error") - | . as $m - | ($m.spans[] | select(.is_primary)) as $s - | select(any($changed[]; . == $s.file_name)) - | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" - ' "/tmp/clippy-${crate}.json" 2>/tmp/jq-err)" - if [[ -s /tmp/jq-err ]]; then - cat /tmp/jq-err >&2 - echo "jq failed to read the clippy output" >&2 - exit 1 - fi - if [[ -n "$offenders" ]]; then - echo "$offenders" - echo "clippy findings in files this PR changes" >&2 - exit 1 - fi - done + # The gate is by changed file path, so it reads the whole stream once. + # Splitting per crate first drops every lint from a crate whose + # package id omits the name (cargo does that when the name equals the + # directory's last segment: codex-home, codex-api, ...). + rm -f /tmp/jq-err + offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' + select(.reason == "compiler-message") + | .message + | select(.level == "warning" or .level == "error") + | . as $m + | ($m.spans[] | select(.is_primary)) as $s + | select(any($changed[]; . == $s.file_name)) + | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" + ' /tmp/clippy.json 2>/tmp/jq-err)" + if [[ -s /tmp/jq-err ]]; then + cat /tmp/jq-err >&2 + echo "jq failed to read the clippy output" >&2 + exit 1 + fi + if [[ -n "$offenders" ]]; then + echo "$offenders" + echo "clippy findings in files this PR changes" >&2 + exit 1 + fi - name: cargo test shell: bash