diff --git a/.github/scripts/fork-changed-packages.py b/.github/scripts/fork-changed-packages.py index b7cdcf082705..d5eed743e866 100644 --- a/.github/scripts/fork-changed-packages.py +++ b/.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) diff --git a/.github/scripts/fork-lock-affected.py b/.github/scripts/fork-lock-affected.py new file mode 100644 index 000000000000..07c050923d41 --- /dev/null +++ b/.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) diff --git a/.github/scripts/test_fork_changed_packages.py b/.github/scripts/test_fork_changed_packages.py new file mode 100644 index 000000000000..be4773d3fbf8 --- /dev/null +++ b/.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() 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 7e0dac9d6428..1a532e1da686 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,58 @@ 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") + 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 + # 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 - # --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 <<< "${{ steps.crates.outputs.list }}" + 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 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 @@ -235,8 +296,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