fork-ci: one cargo invocation per step; Cargo.lock selects only affected packages

.github/scripts/fork-changed-packages.py

@@ -4,29 +4,140 @@
 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.
+
+    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", []))
+    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_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:
+        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)

.github/scripts/fork-lock-affected.py

@@ -0,0 +1,82 @@
+"""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
+
+
+# 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."""
+    graph = edges(meta)
+    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 graph:
+            return frozenset()
+        stack.add(package_id)
+        found = {package_id}
+        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]
+
+    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)

.github/scripts/test_fork_changed_packages.py

@@ -0,0 +1,133 @@
+"""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__)))
+
+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
+
+
+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 fixture(name):
+    with open(os.path.join(HERE, "testdata", name), encoding="utf-8") as handle:
+        return json.load(handle)
+
+
+NODEPS = fixture("nodeps.json")
+ROOT = NODEPS["workspace_root"]
+
+
+class SelectTest(unittest.TestCase):
+    def select(self, paths):
+        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"])
+
+    def test_workspace_manifest_selects_everything(self):
+        self.assertEqual(
+            self.select(["codex-rs/Cargo.toml"]),
+            ["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-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"])
+
+
+class LockTest(unittest.TestCase):
+    def test_lock_only_change_selects_the_affected_subset(self):
+        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,
+                    }
+                ],
+            },
+        }
+        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__":
+    unittest.main()