chore: decompose cargo-hack feature sweep into per-crate parallel targets#2741
Draft
benfdking wants to merge 3 commits into
Draft
chore: decompose cargo-hack feature sweep into per-crate parallel targets#2741benfdking wants to merge 3 commits into
benfdking wants to merge 3 commits into
Conversation
Replace the monolithic `cargo hack check --each-feature` Bazel action with one `cargo check` target per feature per crate, generated from a per-crate hack.bzl variation map via the new cargo_hack_suite macro. This lets Bazel run the feature checks in parallel and attribute failures per feature. Each crate owns a hack.bzl declaring its FEATURES; the //:hack_reconcile test diffs the union of those maps against `cargo hack ... --print-command-list` (enumerated without compiling) so the decomposition stays a one-to-one match with cargo-hack and no feature combination is silently dropped. The //:cargo_hack_check test_suite aggregates the whole sweep plus the reconciliation guard, preserving the existing target name for CI. https://claude.ai/code/session_01LtWNfZpPpUfbW9xuGPrrSy
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
benfdking
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the existing monolithic Bazel cargo hack check --each-feature verification into per-crate, per-feature Bazel test targets, with a root-level reconciliation test to ensure the decomposed enumeration matches cargo-hack’s own --print-command-list output.
Changes:
- Added
each_feature,hack_command, andcargo_hack_suitehelpers to generate per-featurecargo checktests and an aggregating suite. - Introduced per-crate
hack.bzlmodules declaringMANIFEST,FEATURES, and derivedHACKvariation maps. - Updated root and crate
BUILD.bazelfiles to wire per-crate hack suites and add//:hack_reconcile, plus adjusted visibility for shared filegroups/providers.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cargo_build.bzl |
Adds helpers/macro to generate per-feature cargo check tests and suites. |
BUILD.bazel |
Aggregates per-crate hack suites; adds reconciliation test; makes shared cargo filegroups/providers public. |
crates/cli/hack.bzl |
Declares feature list + manifest for per-feature variations. |
crates/cli/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/cli-lib/hack.bzl |
Declares feature list + manifest for per-feature variations. |
crates/cli-lib/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/cli-python/hack.bzl |
Declares featureless crate variation map + manifest. |
crates/cli-python/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lib/hack.bzl |
Declares feature list + manifest for per-feature variations. |
crates/lib/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lib-core/hack.bzl |
Declares feature list + manifest for per-feature variations. |
crates/lib-core/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lib-dialects/hack.bzl |
Declares dialect feature list + manifest for per-feature variations. |
crates/lib-dialects/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lib-wasm/hack.bzl |
Declares featureless crate variation map + manifest. |
crates/lib-wasm/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lineage/hack.bzl |
Declares featureless crate variation map + manifest. |
crates/lineage/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/lsp/hack.bzl |
Declares featureless crate variation map + manifest. |
crates/lsp/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
crates/sqlinference/hack.bzl |
Declares featureless crate variation map + manifest. |
crates/sqlinference/BUILD.bazel |
Adds cargo_hack_suite(name="hack", ...) for per-feature checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Benchmark for 1663ac9Click to view benchmark
|
Each crate's hack targets now take only their dependency-closure sources as action inputs (the crate plus its transitive in-workspace deps, declared as CLOSURE in hack.bzl), instead of the whole workspace. cargo_hack_suite trims the workspace members/default-members to the same closure in-sandbox so cargo loads only those crates rather than validating every member. This isolates Bazel cache keys per closure: editing an unrelated crate (e.g. lineage) no longer invalidates another crate's feature checks (e.g. lib-dialects), maximising cache hits while still checking every feature. https://claude.ai/code/session_01LtWNfZpPpUfbW9xuGPrrSy
Contributor
Benchmark for 5998b03Click to view benchmark
|
Contributor
Benchmark for c98bc2aClick to view benchmark
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the monolithic
cargo hack check --each-featureBazel test into a decomposed, parallelizable suite of per-crate, per-feature targets. Instead of one all-or-nothing serial action, each crate owns its feature variations in ahack.bzlfile, and Bazel generates individualcargo checktargets with better failure attribution and caching.Key Changes
New
cargo_build.bzlhelpers:each_feature(features): builds the--each-featurevariation map for a cratehack_command(manifest, args): formats a single cargo-hack-equivalent command linecargo_hack_suite(name, manifest, variations, closure, vendor, size): generates onecargo checktest per feature variation plus a public aggregating suiteNew
hack.bzlfiles in each crate:MANIFEST,FEATURES, andCLOSUREHACKmap viaeach_feature(FEATURES)Updated crate
BUILD.bazelfiles:cargo_hack_suiteand each crate'shack.bzlcargo_hack_suite(name = "hack", ...)bazel test //crates/<name>:hackRoot
BUILD.bazelchanges://:hack_reconcileto verify the decomposed maps matchcargo hack check --each-feature --print-command-listcargo_hack_checkwith atest_suiteaggregating all per-crate hack suites plus the reconciliation guardReadiness Fixes
:hacksuites are explicitly public so the root//:cargo_hack_checksuite can depend on them.//:hack_reconcileis sized asmedium; it exceeded thesmalltimeout locally while producing the command list.Validation
bazel test //:hack_reconcile --test_output=errorsbazel test //:cargo_hack_check --test_output=errorsgit diff --checkThe full aggregate cargo-hack suite passes locally: 41/41 targets green.