From 11251d4de10e77874e897d3189ad7eef23cc2503 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 18:57:50 +0200 Subject: [PATCH 01/13] plan: add targets to feature combinations --- plan/per-target-feature-combinations.md | 1637 +++++++++++++++++++++++ 1 file changed, 1637 insertions(+) create mode 100644 plan/per-target-feature-combinations.md diff --git a/plan/per-target-feature-combinations.md b/plan/per-target-feature-combinations.md new file mode 100644 index 0000000..7b6cdf4 --- /dev/null +++ b/plan/per-target-feature-combinations.md @@ -0,0 +1,1637 @@ +# Per-Target Feature Combinations + +## Purpose + +`cargo-fc` already owns the feature-combination axis and already understands +target-specific configuration overrides through +`[package.metadata.cargo-fc.target.'cfg(...)']`. The missing piece is letting a +workspace declare the target triples that should be checked by default, so a +plain local command such as `cargo fc check` exercises the same target cfg views +that CI exercises. + +This plan adds a target axis to the existing feature matrix: + +```text +selected packages x effective targets for each package x feature combinations +``` + +The implementation should keep local and CI behavior aligned, preserve existing +single-target behavior when no target list is configured, and keep Cargo output +live by running one Cargo invocation at a time in v1. + +## Recommendation + +Implement this in staged milestones: + +1. Target planning and precomputed execution plans. +2. Serial per-target execution and matrix output. +3. Opt-in aggregate execution (`--aggregate-targets`). +4. Documentation and compatibility follow-through. + +Precomputed plans should come before execution because they separate feature +resolution from command execution and make target-specific package selection +testable. + +cargo-fc never spawns concurrent Cargo processes; it stays single-threaded and +lets Cargo parallelize within each invocation. v1 ships two execution modes over +the same plans, both serial and both with live output: + +- default: one Cargo invocation per `(package, target, combo)`, giving exact + per-target attribution (PASS/FAIL, diagnostics, dedupe), +- opt-in `--aggregate-targets`: one Cargo invocation per `(package, combo)` that + passes every target sharing that combo as repeated `--target` flags, letting + Cargo overlap the targets' build graphs. Faster on many-core machines, but it + attributes results at group rather than per-target granularity. + +A worker pool of concurrent Cargo processes was measured and rejected; see +"Target Execution Modes" for the numbers. + +## Design Principles + +- Keep target selection separate from target-specific config resolution. Target + lists decide which targets are visited; `target.'cfg(...)'` sections decide + the effective feature matrix for one concrete target. +- Keep planning separate from execution. Resolve package configs, target + overrides, feature combinations, and pruning before running Cargo. +- Keep cargo-fc single-threaded and its output live. Both execution modes stream + Cargo output directly; there is no worker pool, output buffering, or replay. +- Preserve deterministic cargo-fc order. Serial mode reports target plan order, + then package order, then feature-combination order. Aggregate mode reports + package order, then canonical feature-combination order, then target-group + order. +- Do not guess that arbitrary cargo aliases are link-free. Unknown aliases need + explicit opt-in before configured target lists apply to them. + +## First-Class Feature Contract + +The first complete implementation should provide: + +- workspace-level target lists, +- package-level target lists and package-level opt-out, +- `cargo fc matrix` rows with `target`, +- configured multi-target execution for built-in Cargo subcommands with target + capability and for aliases explicitly allowlisted for target support in + config, +- clear fallback behavior for aliases that lack target capability, +- a default serial per-target mode and an opt-in `--aggregate-targets` mode, + both single-threaded with live output, +- deterministic cargo-fc invocation order in both modes, with exact per-target + attribution in the default serial mode. + +Package-level targets should not be a partial bolt-on. They should be part of +the execution plan model from the start. + +## Existing Code Shape + +Relevant current structure: + +- `src/config.rs` + - `Config` stores per-package cargo-fc config. + - `Config::target` stores cfg-keyed target override sections. + - `WorkspaceConfig` currently stores workspace-wide `exclude_packages`. +- `src/workspace.rs` + - Reads `[workspace.metadata.cargo-fc]`. + - Applies workspace and root-package package exclusions. +- `src/target.rs` + - Detects one effective target from `--target`, `CARGO_BUILD_TARGET`, or host. +- `src/cfg_eval.rs` + - Evaluates cfg expressions for a concrete target using + `rustc --print cfg --target `. +- `src/lib.rs` + - Detects one target and dispatches to one-target matrix/run functions. +- `src/runner.rs` + - `print_feature_matrix_for_target(...)` + - `run_cargo_command_for_target(...)` + - Per-target config resolution already happens inside these functions. + +The important architectural point: target-specific config resolution already +works for one target. This change should add planning/orchestration around that, +not duplicate the target override logic. + +## Proposed Architecture + +This should be a full-featured target axis with a small, clean architecture. Do +not weaken the behavior to keep the patch small; keep the behavior complete by +putting each concern in the right place. + +Use a ports-and-adapters style boundary: + +- domain structs describe target selection and execution plans, +- adapters read Cargo metadata, CLI args, environment, and rustc, +- the runner consumes plans and writes results, +- config resolution remains a pure "base config + target -> effective config" + concern. + +Keep the flow as a direct extension of the current architecture: + +```text +CLI args + cargo metadata + -> selected packages + -> target plans + -> per-target config resolution + -> feature-combination execution +``` + +The new first-class concept is a target plan. It says "run these package-target +assignments for this target triple." The plan is deduplicated by target triple +for stable scheduling and output, while each package assignment carries where +that package's target came from. The plan should not know how to generate +feature combinations. + +Suggested domain types: + +```rust +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TargetSource { + Cli, + PackageConfig, + WorkspaceConfig, + CargoBuildTargetEnv, + Host, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EffectiveTarget { + pub triple: TargetTriple, + pub source: TargetSource, +} + +pub struct PlannedPackage<'a> { + pub package: &'a cargo_metadata::Package, + /// Cached base cargo-fc config for this package, loaded once before + /// planning. Carried here so execution-plan construction never re-reads + /// the manifest, which would duplicate deprecation warnings and reparse + /// metadata. + pub config: &'a crate::config::Config, + pub target: EffectiveTarget, +} + +pub struct TargetPlan<'a> { + pub target: TargetTriple, + pub packages: Vec>, +} + +pub struct TargetPlans<'a> { + pub plans: Vec>, + pub contains_configured_assignments: bool, +} +``` + +`contains_configured_assignments` is `true` when target selection was influenced +by configured target metadata or an explicit `--target` (i.e. anything other +than the implicit host/`CARGO_BUILD_TARGET` single-target fallback). This +includes package `targets = []` opt-out, even if the resulting concrete target +source is `Host` or `CargoBuildTargetEnv`. Its only consumer is output +formatting: it gates whether per-entry summaries show the `target = ...` column, +so the default single-host run keeps its current output. It must not be used to +decide whether to warn about skipped configured targets; that warning uses raw +config state before planning. + +Suggested module placement: + +- `workspace.rs` reads workspace metadata. + It should expose candidate package discovery separately from applying + workspace package exclusions, because exclusions can become target-specific. +- `package.rs` continues to read package metadata. +- `target.rs` owns target detection, target flag parsing, and target-source + types. +- `target_plan.rs` or `planner.rs` builds `TargetPlans` from selected packages, + workspace config, package configs, CLI target info, environment, and host + detection. +- `config::resolve` continues to resolve one package config for one concrete + target. +- `runner.rs` executes target plans. + +Suggested ownership split: + +```text +src/target.rs + TargetTriple, TargetSource, EffectiveTarget, target flag parsing, + host/env target adapters. + +src/target_plan.rs + TargetPlans, TargetPlan, target precedence, package/workspace target list + selection, per-package target source tracking, target-specific workspace + package exclusion, stable dedupe. + +src/runner.rs + ExecutionPlan, PackageExecutionPlan, command execution, live output, + summaries. + +src/config.rs and src/config/resolve.rs + serde config fields and existing target override resolution only. + +src/workspace.rs + workspace metadata parsing, workspace-root warnings, candidate package + discovery, and target-specific workspace exclude resolution helpers. +``` + +If an implementation finds itself adding target-list decisions to +`config::resolve` or feature-combination generation, that is a design smell. +Target lists choose the outer execution axis; target overrides shape the config +inside one selected target. + +Do not push target-list logic into `Package::feature_combinations`. Feature +generation should continue to accept an already-resolved `Config`; the target +axis is one level above that. + +### Ports and Adapters + +Keep side effects behind small traits so the planner is easy to test: + +```rust +pub trait TargetEnvironment { + fn cargo_build_target(&self) -> Option; + fn host_target(&self) -> eyre::Result; +} + +pub struct SelectedPackage<'a> { + pub package: &'a cargo_metadata::Package, + pub config: &'a crate::config::Config, +} +``` + +The exact trait names can differ. The point is that target planning should be +unit-testable without invoking real `rustc` or spawning Cargo. The production +adapter can delegate to the existing `RustcTargetDetector`, +`RustcCfgEvaluator`, and one package config loading pass. + +Planning must also receive a `CfgEvaluator`. Target-specific workspace +`exclude_packages` uses `cfg(...)` keys, so planning needs the same target cfg +data that package target overrides use. The existing evaluator trait and stub +test evaluator are the right boundary; do not make the planner shell out to +`rustc` directly. + +Cache each selected package's base `Config` once before target planning and pass +those cached configs through planning and execution-plan construction. +`Package::config()` emits deprecation warnings for old keys, so calling it once +for target selection and again for feature resolution would duplicate warnings +and do unnecessary metadata parsing. + +Do not hide config loading behind a planner adapter that calls +`Package::config()` on demand. The planner should consume already-loaded +`SelectedPackage`/`PlannedPackage` data. + +Avoid a large abstract framework. Two or three narrow traits are enough if they +make tests deterministic and keep IO out of the planning logic. + +### Planning Invariant + +After target planning, every later stage should receive explicit target plans. +No later stage should need to ask "what targets should I run?" It should only +ask: + +- what concrete target is this plan for? +- which package assignments belong to this target? +- should this package assignment inject `--target` into Cargo args? + +This invariant is what makes the feature feel first-class instead of bolted on. + +### Single-Target Compatibility + +The existing public single-target functions should remain usable. Internally, +they can wrap a one-item `TargetPlans` value. In the sketch below, `packages` +means the already-loaded `(Package, Config)` pairs: + +```rust +TargetPlans { + plans: vec![TargetPlan { + target: target.clone(), + packages: packages.iter().map(|&(package, config)| PlannedPackage { + package, + config, + target: EffectiveTarget { + triple: target.clone(), + source: TargetSource::Cli, // or another caller-provided source + }, + }).collect(), + }], + contains_configured_assignments: false, +} +``` + +That lets new multi-target behavior be first-class without forcing all library +consumers to migrate immediately. + +### Complexity Guardrails + +- Add one target-planning abstraction, not several layers of planners. +- Keep target list selection out of target override resolution. +- Keep concurrency out of config resolution and feature generation. +- Prefer serial correctness first. Aggregate execution must be a small execution + mode over the same plan, not a separate planning path with different + semantics. +- Keep CLI surface small. Do not add concurrency flags in v1. + +## Goals + +- Allow target lists in repo config: + + ```toml + [workspace.metadata.cargo-fc] + targets = [ + "x86_64-unknown-linux-gnu", + "x86_64-pc-windows-msvc", + "aarch64-apple-darwin", + ] + ``` + +- Allow package-level target lists: + + ```toml + [package.metadata.cargo-fc] + targets = ["wasm32-unknown-unknown"] + ``` + +- Compose configured targets with existing target override sections. +- Preserve single-target behavior when no configured target list exists. +- Preserve explicit `--target ` as the strongest override. +- Add `target` to `cargo fc matrix` rows. +- Deduplicate target triples while preserving declaration order. +- Preserve live output by keeping cargo-fc single-threaded in both modes. +- Add an opt-in `--aggregate-targets` flag that batches a combination's targets + into one Cargo invocation (`--target A --target B ...`) for throughput. +- Add a small command target policy so aliases can explicitly opt in to + configured targets and cargo-fc-injected `--target` flags. + +## Non-Goals + +- Do not install Rust targets automatically. If a target is missing, fail with a + clear `rustup target add ` hint. +- Do not guarantee that cross-target `build`, `test`, or `run` succeeds. These + commands may require linkers, runners, or target OS support. cargo-fc can pass + supported flags; the toolchain prerequisites remain the user's + responsibility. +- Do not spawn concurrent Cargo processes or add a worker pool, thread pool, or + `--max-concurrent-targets`-style concurrency. Both execution modes are + single-threaded; aggregate mode delegates parallelism to a single Cargo + invocation. Feature-combination-level parallelism is also out of scope. +- Do not change the semantics of existing cfg override sections. +- Do not change existing `--diagnostics-only` or `--dedupe` command eligibility + in v1. Preserve today's best-effort behavior for `test`, `run`, and aliases. + +## Command Scope + +Configured target lists should apply only when the selected cargo subcommand has +the target capability. Built-in Cargo subcommands get this capability from a +small built-in registry. Unknown aliases get no target capability unless the +workspace config opts them in. + +This avoids guessing that `cargo lint` means `cargo clippy`. `lint` could be any +Cargo alias or custom command, so it must remain unknown unless the workspace +explicitly declares that its `lint` command accepts Cargo-style `--target`. + +Do not extend this v1 policy to generated feature-selection flags, jobs, +message-format injection, or diagnostics parsing: + +- generated feature-selection flags should preserve the current behavior: + known cargo subcommands are normalized, and unknown aliases keep the existing + legacy best-effort path, +- there is no target-level parallelism in v1, so cargo-fc should not compute or + inject `--jobs`, +- `--diagnostics-only` and `--dedupe` should preserve today's behavior for + `test`, `run`, and aliases. Do not disable them through the target capability + policy. + +Known built-in commands should have explicit built-in target allowlist entries. +The allowlist is not only for Cargo's built-in binaries; it is for command +tokens that cargo-fc knows how to reason about. For example, `clippy` must be in +the built-in allowlist because cargo-fc knows `cargo clippy` accepts Cargo's +`--target` flag. + +Encode the table below as the initial built-in registry. If implementation +tests prove that a command does not actually accept `--target`, adjust the +registry and add a regression test explaining the exception. + +The initial built-in registry should track the command tokens cargo-fc already +recognizes today: `build`, `check`, `clippy`, `test`, `doc`, and `run`, plus +their existing short aliases where applicable. Do not add mutating or +non-matrix-oriented Cargo commands such as `fix` as built-ins in this plan. If +cargo-fc later starts recognizing another safe Cargo subcommand, add a built-in +target-capability entry and tests at the same time. + +```text +subcommand target capability +check yes +clippy yes +build yes +doc yes +test yes +run yes +unknown no +matrix yes +``` + +For `cargo fc matrix`, target capability means "use configured target planning +when generating rows." It does not imply any Cargo flag injection because no +Cargo command is spawned. + +Short built-in aliases should resolve to the same target capability as their long +forms when cargo-fc already recognizes them, for example `c` -> `check`, +`b` -> `build`, `t` -> `test`, `d` -> `doc`, and `r` -> `run`. + +Unknown aliases should retain the existing best-effort feature-matrix behavior +for generated feature-selection flags unless the implementation intentionally +makes a breaking change in a future release. This preserves the current cargo-fc +contract for aliases that already work. The new stricter allowlist applies only +to configured target lists and cargo-fc-injected `--target`. + +Do not infer alias expansion from the alias name. In particular, do not treat +`lint` as `clippy` just because many projects use that convention. A workspace +that wants `cargo fc lint` to receive configured targets should configure: + +```toml +[workspace.metadata.cargo-fc.subcommands.lint] +targets = true +``` + +Current code names the detected `clippy` command `CargoSubcommand::Lint`. +During implementation, either rename that enum variant to `Clippy` or keep the +variant but make the capability registry/display name clearly refer to the +literal `clippy` subcommand. The literal command token `lint` must stay +unknown unless workspace config opts it in. + +`test` and `run` can accept Cargo `--target`, so they should be allowed for the +target capability. Their existing diagnostics-only behavior must not change in +v1. + +`build`, `test`, and `run` may still fail for foreign targets if the user has +not installed linkers, runners, or platform support. That is acceptable: if a +repo configures targets and invokes a command with target capability, cargo-fc +should faithfully pass the target axis to Cargo and let Cargo/toolchain errors +surface clearly. + +Note that the workspace `targets` list is shared across all subcommands. It is +motivated by `check`/`clippy`, but it also applies to `build`, `test`, and `run` +because they carry target capability. The failure gradient differs sharply: +cross-target `check`/`clippy`/`doc` usually succeed (they only need the target's +`rustc`), `build` needs a linker, and `test`/`run` actually execute and so +usually fail for foreign targets. This is a foot-gun: a repo that configures +`targets` for linting will, by default, make `cargo fc test` attempt every +configured triple. v1 keeps the shared list for simplicity; the escape hatch is +explicit `--target ` to narrow a single run. This must be documented +prominently. Per-subcommand target lists are a possible future refinement only +if users hit this in practice. + +For an unknown command or alias: + +- do not inject configured target flags unless `targets = true` is configured, +- keep existing best-effort feature-matrix behavior for generated feature flags + unless and until a separate breaking-change plan changes alias semantics, +- keep existing diagnostics-only and dedupe behavior, +- when configured targets are skipped, emit one actionable warning that explains + the target capability was skipped and how to opt in. + +Do not warn merely because a subcommand is unknown. Warn only when cargo-fc is +skipping behavior that the user requested or configured: + +- configured target lists exist, but the command lacks `targets`, +- a workspace target-capability entry is malformed or refers to a built-in + command. + +Warnings must be emitted once per invocation, not once per package, target, or +feature combination. The warning should include the detected raw subcommand +token and the exact config snippet needed to opt in. + +### Capability Resolution + +Resolve command target capability once, immediately after parsing cargo-fc flags +and before target planning: + +1. Detect the raw cargo subcommand token using the same cargo-flag skipping + rules currently used by `cargo_subcommand`. This needs a token-extracting + helper because the current enum collapses all unknown commands to `Other`. +2. If the token maps to a known built-in cargo subcommand, use the built-in + target-capability registry. +3. Otherwise, look up `[workspace.metadata.cargo-fc.subcommands.]`. +4. If no configured entry exists, deny configured targets for that command. + +The `cargo fc matrix` command is not a forwarded cargo subcommand: it has target +capability unconditionally and skips token detection entirely. The resolution +steps above apply only to forwarded cargo commands. + +Represent the result as a value, not scattered checks: + +```rust +pub struct ResolvedCommandTargetPolicy { + pub command_name: String, + pub source: CommandCapabilitySource, + pub targets: CapabilityDecision, +} + +pub enum CommandCapabilitySource { + BuiltIn, + WorkspaceConfig, + Unknown, +} + +pub enum CapabilityDecision { + Allowed, + Denied, +} +``` + +The exact type names can differ. The important design rule is that the runner +consumes one resolved policy object instead of repeatedly asking whether a +subcommand is known. + +Built-in commands should not require workspace config. If a workspace config +entry uses the same name as a built-in command, either reject it or warn that it +is ignored. Do not silently let a config table change built-in command behavior +in the first implementation; that would make standard cargo-fc behavior harder +to reason about. + +The target warning must be driven from raw config state, not from the planned +targets after capability filtering. Before planning, compute whether any +workspace or selected package declares a non-empty target list. If configured +targets exist and the selected command lacks target capability, emit the warning +and build the normal single-target plan. + +Future work may introduce a broader capability policy for diagnostics or other +cargo-fc-injected flags. If it does, it must be treated as a compatibility +change: `test`, `run`, and aliases currently work on a best-effort basis with +`--diagnostics-only`/`--dedupe`, and that behavior should not be removed without +an explicit migration path. + +## Configuration Schema + +All new metadata keys should work under the existing metadata aliases: + +- `[workspace.metadata.cargo-fc]` +- `[workspace.metadata.fc]` +- `[workspace.metadata.cargo-feature-combinations]` +- `[workspace.metadata.feature-combinations]` +- matching `[package.metadata.*]` sections + +Follow the existing alias precedence instead of introducing target-specific +lookup rules. + +### Workspace Config + +Add fields to `WorkspaceConfig`: + +```rust +pub struct WorkspaceConfig { + pub exclude_packages: HashSet, + #[serde(default)] + pub targets: Vec, + #[serde(default)] + pub target: BTreeMap, + #[serde(default)] + pub subcommands: BTreeMap, +} + +#[derive(Debug, Clone, Default, Deserialize, Serialize)] +pub struct WorkspaceTargetOverride { + pub exclude_packages: Option, +} +``` + +Suggested defaults: + +- `targets = []` +- `target = {}` +- `subcommands = {}` + +An empty workspace target list means "no configured target list"; behavior falls +back to the existing effective target detection path. + +Workspace-only keys should be read only from the workspace root, matching the +current `exclude_packages` behavior. This includes `targets`, workspace +`target.'cfg(...)'` overrides, and `subcommands`. If a non-root package appears +to contain one of these +`[workspace.metadata.cargo-fc]` keys, warn that workspace metadata is only read +from the workspace root. + +### Target-Specific Workspace Package Selection + +Allow workspace package exclusions to vary by target: + +```toml +[workspace.metadata.cargo-fc] +targets = [ + "x86_64-unknown-linux-gnu", + "wasm32-unknown-unknown", +] + +[workspace.metadata.cargo-fc.target.'cfg(target_arch = "wasm32")'] +exclude_packages = { add = ["native-cli"] } + +[workspace.metadata.cargo-fc.target.'cfg(target_os = "linux")'] +exclude_packages = { add = ["wasm-app"] } +``` + +This is intentionally narrow. Workspace target overrides may patch +`exclude_packages` only. Do not allow these sections to change `targets`, +`subcommands`, or command target capability. Target lists choose the outer axis; +workspace target overrides only decide which workspace packages participate for +one already-selected target. + +Use the same patch semantics as package target overrides for set-like fields: + +- array syntax is an override, +- `{ override = [...] }` replaces the base set, +- `{ add = [...] }` unions with the base set, +- `{ remove = [...] }` subtracts from the base set, +- matching cfg sections merge deterministically by cfg key order, +- conflicting overrides are errors. + +Use the same cfg evaluator and validation rules as package target overrides. +In particular, `cfg(feature = "...")` must remain unsupported in workspace +target override keys; these sections select by target cfg only. + +The effective excluded package set for one target is: + +```text +workspace.exclude_packages + + deprecated root-package exclude_packages + patched by matching [workspace.metadata.cargo-fc.target.'cfg(...)'] +``` + +Then that effective set is applied to the target's package list. + +Workspace target overrides apply to every concrete effective target, including +single-target invocations selected by explicit `--target`, `CARGO_BUILD_TARGET`, +or host fallback. They are not limited to configured multi-target runs. + +Implementation impact: split workspace package discovery from workspace package +exclusion. `packages_for_fc()` currently filters `exclude_packages` globally, +but target-specific exclusions require target planning to start from candidate +workspace packages and apply the effective exclude set per target. Preserve the +existing deprecation warnings for root-package `exclude_packages`; just fold +those values into the base exclude set before target-specific patches. + +### Command Target Capability Config + +Add a workspace-level target-capability table for aliases and custom cargo +subcommands: + +```toml +[workspace.metadata.cargo-fc.subcommands.lint] +targets = true +``` + +This means: when the detected cargo subcommand is `lint`, cargo-fc is allowed to +expand configured target lists and inject `--target `. + +This table must not change generated feature-selection behavior or diagnostics +behavior in v1. + +Suggested Rust shape: + +```rust +#[derive(Debug, Clone, Default, Deserialize, Serialize)] +pub struct CommandTargetCapability { + #[serde(default)] + pub targets: bool, +} +``` + +A plain `bool` (default `false`) is enough in v1. Only unknown aliases consult +this table and the default is deny, so "omitted" and "`targets = false`" are +indistinguishable in practice; `Option` would add a distinction with no +observable effect. If a future version grows more capabilities with non-deny +defaults, switch the relevant fields to `Option` then. + +For unknown aliases the default is therefore `targets = false` (denied). +Built-in commands get their capability from code and ignore this table. + +Keep this table in workspace metadata only. Command aliases are an invocation +property, not a package property. Per-package command target capability would +make a single `cargo fc lint` invocation ambiguous when selected packages +disagree. + +Built-in target capability should be provided by code. Workspace config is for +aliases/custom commands only. Do not require users to configure standard Cargo +subcommands such as `check`, `clippy`, or `build`, and do not let workspace +config silently override built-in command behavior in the first implementation. + +The `targets` capability name describes cargo-fc behavior: cargo-fc may expand +configured target lists and add `--target `. + +When configured targets exist and the selected command lacks target capability, +cargo-fc should skip the target expansion and warn. For example: + +```text +warning: not passing --target to cargo alias `lint` because it has no configured targets capability +hint: add [workspace.metadata.cargo-fc.subcommands.lint] targets = true if this alias accepts --target +``` + +### Package Config + +Add package-level targets to `Config`: + +```rust +pub struct Config { + #[serde(default)] + pub targets: Option>, + // existing fields... +} +``` + +Use `Option>`, not `Vec`, so the implementation can +distinguish these cases: + +- missing `targets`: inherit workspace target list +- `targets = []`: explicit package-level opt-out of workspace targets, using + the fallback single effective target instead +- `targets = ["..."]`: package-specific target list + +This matters for mixed workspaces where most packages should run on the +workspace target list, but one package is host-only or wasm-only. + +`targets` is a selection field, not a feature-matrix field. Do not add it to +`TargetOverride`, and do not allow `[package.metadata.cargo-fc.target.'cfg(...)']` +to change the target list. Target override sections are evaluated after a +concrete target has already been selected. + +If `Config` is cloned by `config::resolve`, either preserve `targets` +harmlessly or clear it from the resolved config. The critical rule is that +`Package::feature_combinations` must not read `targets`. + +### CLI Flags + +Add exactly one new cargo-fc flag in v1: `--aggregate-targets`. It is a boolean +flag, drained before forwarding args to Cargo (like `--summary-only` and the +other cargo-fc flags), and selects aggregate execution mode (one Cargo +invocation per `(package, combo)` carrying all that combo's targets). The default +(flag absent) is serial per-target. There are no concurrency flags. + +The existing forwarded Cargo `--target` flag becomes more important because it +overrides configured target lists. Parse it only from forwarded Cargo args +before `--`; arguments after `--` belong to tests or binaries and must not +affect cargo-fc target planning. This fixes the latent case: + +```bash +cargo fc run -- --target value-for-the-binary +``` + +The value above must not be treated as Cargo's target triple. + +## Target Precedence + +Target planning must be capability-aware. If the selected command lacks the +`targets` capability, configured workspace/package target lists are ignored for +that invocation and cargo-fc falls back to the current single effective target +path: explicit CLI `--target`, then `CARGO_BUILD_TARGET`, then host. + +When the selected command has the `targets` capability, target planning should +use this precedence for each selected package: + +1. Explicit Cargo CLI `--target ` or `--target=`. +2. Package-level `targets`, when present for that package. +3. Workspace-level `targets`, when non-empty. +4. `CARGO_BUILD_TARGET`. +5. Host target from `rustc -vV`. + +This is intentionally different from the current single-target detector once a +repo has configured targets. Repository config should not be silently collapsed +by a developer's ambient `CARGO_BUILD_TARGET`. This is also intentionally +different from Cargo's own precedence for `[build].target`: for cargo-fc, +configured workspace/package target lists are the declarative matrix. If a user +wants one target for a run, explicit `--target ` is the override. This +must be documented because users familiar with Cargo may expect +`CARGO_BUILD_TARGET` to beat file config. + +When explicit `--target` is present, it wins globally and all package/workspace +configured target lists are ignored for that invocation. + +Every package-target assignment must carry its `TargetSource`. This keeps later +decisions simple: + +- `Cli`: Cargo already received the target flag from the user. +- `PackageConfig` or `WorkspaceConfig`: cargo-fc must inject `--target`. +- `CargoBuildTargetEnv`: Cargo will see the env var; cargo-fc does not need to + inject a target. +- `Host`: keep current no-`--target` behavior. + +A target plan itself is deduplicated by triple, not by source. This matters when +the same target triple is reached through different precedence levels for +different packages, for example when one package inherits a workspace target and +another package uses `targets = []` and falls back to the host target. The +runner must decide target injection per package execution from that package's +`EffectiveTarget::source`, not from a plan-wide source. + +This source field avoids boolean drift like `is_configured`, `should_inject`, +and `is_multi_target` scattered through the runner. + +Prefer methods on the enum for repeated policy checks: + +```rust +impl TargetSource { + fn should_inject_target_arg(self) -> bool { + matches!(self, Self::PackageConfig | Self::WorkspaceConfig) + } + + fn is_configured(self) -> bool { + matches!(self, Self::PackageConfig | Self::WorkspaceConfig) + } +} +``` + +This keeps policy near the type that owns the meaning. + +## Effective Target Planning + +Introduce a planning layer before calling the runner. + +Top-level flow should be: + +1. Discover candidate workspace packages without applying workspace + `exclude_packages`. +2. Apply CLI package selection filters. +3. Load and cache each selected package's base cargo-fc config once. +4. Resolve target plans for those candidate packages. +5. Apply the effective workspace `exclude_packages` set per target plan. + +This is a larger refactor than simply adding a target loop. Today package +discovery/exclusion happens before target detection, but target-specific +workspace exclusions apply even for explicit single-target invocations. + +Build `TargetPlans` from the candidate packages in two passes: + +1. Resolve the effective target list for each selected package using cached + package config. +2. Deduplicate triples within each package list while preserving order. +3. Build the global target order: + - explicit CLI target: the single CLI target, + - workspace targets: workspace target order first, + - package-only targets: first-seen order by selected package order and then + by that package's target list, + - fallback target: the single `CARGO_BUILD_TARGET` or host target. +4. For each target in the global order, attach selected packages whose effective + target list contains that target, preserving each package's `TargetSource` + for that target. +5. Resolve the effective workspace `exclude_packages` set for that target and + remove excluded package assignments from that target plan. +6. Drop empty target plans. + +This supports package-level target lists without requiring the runner to process +all packages for all targets. Deduplicate target triples for scheduling and +output, but do not discard per-package target source information. + +Example: + +```toml +[workspace.metadata.cargo-fc] +targets = ["linux", "windows"] + +[package.metadata.cargo-fc] +targets = ["wasm32-unknown-unknown"] +``` + +If the package above is `web`, and another package inherits the workspace list, +the target plans become: + +```text +linux: inherited packages +windows: inherited packages +wasm32: web +``` + +Use real triples in tests; the names above are illustrative only. + +Target-specific workspace `exclude_packages` is applied after package-level +target selection. That means a package can declare that it supports a target, +while the workspace can still exclude it for that target in a central policy. + +### Package-Level Target Semantics + +Package-level `targets` should override, not merge with, workspace-level +`targets`. This is simpler and less surprising: + +- no package `targets`: inherit workspace targets, +- package `targets = []`: opt out of workspace targets and use the fallback + single effective target, +- package `targets = ["..."]`: run only those package targets. + +Do not add package/workspace target merging in the first implementation. If a +package needs workspace targets plus one extra target, it should list the full +package target set explicitly. That keeps target planning inspectable from the +manifest alone. + +### Target List Validation + +Keep validation intentionally small: + +- trim strings, +- reject empty target triples, +- deduplicate while preserving order. + +Do not try to validate triples against rustc target lists during config parsing. +The authoritative check is the existing `rustc --print cfg --target ` +path and the eventual Cargo invocation. If a target is not installed, surface a +clear hint. + +### Target Availability Errors + +Do not add a separate "validate all installed targets" preflight in the first +implementation. It would duplicate rustc/Cargo behavior and add another slow +toolchain call. + +Instead, classify failures at the existing adapter boundaries: + +- when `RustcCfgEvaluator` runs `rustc --print cfg --target `, +- when a Cargo command fails before emitting useful diagnostics because a target + is missing. + +Wrap recognized missing-target failures with: + +```text +target `` is not installed +hint: run `rustup target add ` +``` + +Keep the original error as context. Do not hide non-target toolchain failures +behind the rustup hint. + +## Injecting Cargo Targets + +Today, when `--target` is provided by the user, Cargo sees it because the +original arg is forwarded. For configured targets, the runner must add +`--target ` to each spawned Cargo command that corresponds to a +configured package-target assignment. + +Rules: + +- If the package assignment came from user-provided CLI `--target`, do not + inject another target. +- If the package assignment came from package/workspace config and the command + has the `targets` capability, inject `--target `. +- If a package assignment came from package/workspace config but the command + lacks the `targets` capability, target planning should not have produced a + configured multi-target assignment. Treat that as a planner bug. +- If the package assignment came from `CARGO_BUILD_TARGET`, do not inject; Cargo + will see the environment variable. +- If the package assignment came from host detection, keep current + no-`--target` behavior. + +Make the injection decision per package execution, not per target plan. A +single target plan can contain package assignments with different sources when +the same triple appears through multiple precedence paths. Configured package +assignments are only valid for commands with the `targets` capability. + +## Matrix Output + +Change `cargo fc matrix` so every row includes `target`: + +```json +{ + "name": "my-crate", + "target": "x86_64-pc-windows-msvc", + "features": "serde,cli" +} +``` + +Merge order should preserve user metadata behavior: + +1. Start with `config.matrix`. +2. Merge built-in fields: `name`, `target`, `features`. + +Built-in fields should win if the user supplies the same keys in +`config.matrix`; otherwise matrix consumers can receive misleading rows. +Because `target` becomes a reserved built-in matrix key, emit a compatibility +warning if package matrix metadata already contains `target`. This is a schema +change for existing `cargo fc matrix` consumers and must be called out in the +release notes and final README docs. + +For `--packages-only`, emit one row per package-target pair with +`features = "default"` as today. + +## Summary Output + +Serial per-target execution gives exact per-target attribution: every diagnostic, +PASS/FAIL, and dedupe note belongs to exactly one Cargo invocation, so cargo-fc +always knows which target produced it. Surface that attribution in two places. + +First, the per-combination command/progress header (printed before each Cargo +invocation, ahead of its streamed diagnostics) must include the target, so that +diagnostics streamed under it tell the user which target they belong to: + +```text + Checking my-crate ( target = x86_64-pc-windows-msvc, features = [serde] ) +``` + +Second, add target to each executed summary entry: + +```text +PASS my-crate (target = x86_64-pc-windows-msvc, 0 errors, 0 warnings, features = [serde]) +``` + +Show the `target = ...` field (in both the header and the summary) only when the +run is not the implicit single-host default — that is, when targets are +configured, explicitly selected, or more than one target is planned +(`TargetPlans::contains_configured_assignments`). This keeps the default +single-host run's output unchanged. + +In aggregate mode (`--aggregate-targets`) one invocation covers several targets, +so a summary entry is keyed by `(package, combo, target-group)` and shows +`targets = [t1, t2, ...]` instead of a single `target`. Its PASS/FAIL and counts +are the combined result for that invocation; per-target attribution is not +available for that multi-target group. Singleton groups should keep the serial +shape (`target = ...`) because exact attribution is still available and there is +no throughput benefit from making the output less precise. + +Also include target in pruned summaries. A feature combination pruned for one +target is not necessarily pruned for another target because target overrides can +change the effective config. To keep v1 simple and correct, +`--aggregate-targets` must fall back to serial per-target execution whenever +pruned summaries are requested, either by CLI `--show-pruned` or by any resolved +package config. Pruned-summary display is inherently per `(package, target)`; +do not invent grouped pruned summaries in v1. + +The global finished line should mention targets when more than one target was +planned: + +```text +Finished 42 feature combinations for 3 packages across 2 targets in 18.20s +``` + +Implementation details that must change: + +- `Summary` needs target display context, not just `Option`. + Suggested shape: + + ```rust + enum SummaryTarget { + Hidden, + Single(TargetTriple), + Group(Vec), + } + ``` + + `Hidden` preserves implicit single-host output, `Single` prints + `target = ...`, and `Group` prints `targets = [...]`. +- The per-combination header (`print_package_cmd`) must take the target and + print the same target display context when applicable, so streamed diagnostics + in `--diagnostics-only` mode are attributable to a target or aggregate target + group. +- Serial summary counting must key executed combinations by `(package, target, + features)`, not `(package, features)`, or identical feature sets across + targets will collapse. +- Aggregate summary counting must key executed combinations by `(package, + canonical-combo, target-group)`. Counts are group-summed from the single Cargo + invocation and are not directly comparable to serial per-target counts. +- `append_pruned_summaries` must match pruned entries against executed summaries + scoped to the same `(package, target)` in serial mode. If pruned summaries are + enabled, aggregate mode is not used. +- Serial summary sorting must preserve target plan order, then package order, + then feature order. Aggregate summary sorting must preserve package order, + canonical combo order, then target-group order. Do not globally sort a + package's summaries by features if that scrambles either execution mode's + output order. +- Exit-code aggregation across target plans should be deterministic: for + non-fail-fast serial execution, keep running all planned combinations but + return the first failing exit code in execution-plan order. + +## Dedupe Semantics + +In serial per-target mode, `--dedupe` should deduplicate diagnostics across the +full executed product: + +```text +package x target x feature combination +``` + +Current global `seen_diagnostics` behavior can be extended across target plans +in execution-plan order. This means an identical rendered diagnostic that +appears for multiple targets is printed once, attributed to the first planned +target whose combination emits it, and later occurrences are counted as +suppressed duplicates. + +That global behavior is intentional for v1 because it preserves the meaning of +`--dedupe`: reduce repeated diagnostics across the executed product. It has a +tradeoff: a diagnostic common to several targets is not rendered once per +target. Make the target visible in command/progress output and summaries so the +first attribution is inspectable, and document the tradeoff. + +In aggregate mode, counts are for the whole target group, not per target: + +- Without `--dedupe`, a warning common to three targets may be rendered three + times by Cargo inside the one aggregate invocation and counted three times in + that group summary. It will not be labeled per target. +- With `--dedupe`, the existing per-invocation dedup (`seen_this_run`) folds + those repeated rendered diagnostics to one shown diagnostic for the + `(package, combo, group)` invocation. Because they are folded inside one + invocation, the suppressed count can differ from serial mode, where later + targets are suppressed by the global `seen` set. + +The pass/fail verdict is expected to match serial for clean/failing +combinations, but per-entry warning/error/suppressed counts are not guaranteed +to match serial. Aggregate diagnostics are usually most useful with `--dedupe`; +document that recommendation. + +Aggregate mode is deterministic at the cargo-fc invocation level: package order, +combo order, target-group order, and summary ordering are controlled by +cargo-fc. Within one aggregate Cargo invocation, Cargo owns rustc scheduling and +message ordering. Do not promise per-target diagnostic ordering or attribution +inside an aggregate invocation; that is the explicit tradeoff for throughput. + +## Output + +Both modes keep the current live-output model: cargo-fc runs one Cargo invocation +at a time and streams its stdout/stderr (and diagnostics-only output) directly to +the terminal. There is no output buffering, sink abstraction, or replay — those +only existed to serve concurrent workers, which v1 does not have. + +Progress uses the existing scheme: compute the total invocation count up front +from the execution plan, then increment a counter as invocations run in +deterministic order (target-plan order, then package, then combo for serial; +package, then combo, then target-group for aggregate). No preassigned +per-combination indices are needed without parallel replay. + +## Target Execution Modes + +cargo-fc never spawns concurrent Cargo processes. It runs one Cargo invocation at +a time and relies on Cargo's own scheduler to use the cores. v1 has two +single-threaded modes over the same execution plans: + +- **serial per-target (default).** One invocation per `(package, target, combo)`, + passing a single `--target` when the source requires injection. Gives exact + per-target attribution: every PASS/FAIL, diagnostic, and dedupe note belongs to + one target. +- **aggregate (`--aggregate-targets`).** One invocation per `(package, combo)` + that passes every target sharing that combo as repeated `--target` flags. Cargo + builds them together, overlapping their job graphs. Faster on many-core + machines, but attribution is per group, not per target for groups with more + than one target. + +Both stream live output and use deterministic cargo-fc invocation order. Neither +uses threads. Aggregate mode still inherits Cargo's internal diagnostic ordering +inside each multi-target invocation. + +### Why no worker pool + +Running concurrent Cargo processes was measured and rejected. With `cargo 1.96`, +on this repo (`cargo clippy`, two targets, full rebuilds via `cargo clean`) and a +dependency-heavy sample crate (`regex`+`serde_json`+`url`); 2-core figures use +`taskset -c 0,1`: + +| approach | 48 cores, 2 targets | 2 cores, 2 targets | attribution | extra cost | +| --- | --- | --- | --- | --- | +| serial per-target (default) | 6.1 s | 11.0 s | exact | none | +| concurrent processes, shared `target/` | 6.0 s (no gain) | ~serial | exact | none | +| concurrent processes, per-target `CARGO_TARGET_DIR` | 4.5 s | 14.2 s (slower) | exact | 2x deps + 2x disk | +| aggregate: one invocation, `--target A --target B` | 4.1 s | 10.8 s | group-level | none | + +- Concurrent processes sharing the default `target/` do not parallelize: Cargo + locks per `target///.cargo-lock` and also takes the + host `target//.cargo-lock` for build scripts/proc-macros/host deps, so + two `--target` builds (or two `--features` builds) serialize on the shared host + lock. (Debug vs release escape only because they use different profile dirs; + that does not generalize to triples.) +- Per-`CARGO_TARGET_DIR` workers do parallelize but recompile shared host + artifacts and deps once per target: a modest win on many cores, but 28% slower + than serial on 2 cores, plus 2x disk and the whole buffered-capture / + ordered-replay / job-budgeting apparatus. Net loss for the common small CI + runner. +- A single multi-target invocation (aggregate mode) is fastest and cheapest — one + process, one lock, host artifacts shared once, Cargo fills the tail across + targets (3 targets: 4.1 s vs 7.4 s serial on 48 cores; ~tied on 2 cores, never + slower). Its only cost is attribution (below). + +### Aggregate mode details + +- **Grouping.** For each selected package, group its executed combinations by a + canonical feature-combination key, preferably the sorted `Vec` already + used by the matrix planner rather than a display string. Targets whose + resolved matrix contains a given combo share one invocation for it. + Target-specific `cfg(...)` overrides that change the feature set simply + produce different combo keys and fall into different invocations automatically + — no explicit "feature-matrix grouping" logic is needed beyond keying by the + canonical combo. +- **Injection.** Aggregate mode applies only to configured multi-target runs, so + every batched target has source `PackageConfig`/`WorkspaceConfig` and gets a + `--target `. Explicit-`--target`, env, and host fallbacks are + single-target and run as serial. +- **Attribution.** Cargo's JSON diagnostics carry no triple (a `compiler-message` + is `{reason, package_id, manifest_path, target, message}`, where `target` is the + lib/bin, not the triple) and one invocation returns one exit code. So results + for multi-target groups are reported per `(package, combo, group)` with + `targets = [...]`. Singleton groups should use the normal `target = ...` + output. Group-level attribution is the documented tradeoff for speed; the + default serial mode is the choice for exact per-target attribution everywhere. +- **Command applicability.** Aggregate requires a command that accepts repeated + `--target`. `run` does not (Cargo rejects multiple `--target` for `run`), so + `--aggregate-targets` falls back to serial per-target for `run` with a one-line + note. The observed Cargo error is `error: only one --target argument is + supported`. `test` accepts it (builds all, runs host; foreign-target test + binaries fail to execute, exactly as in serial). `matrix` is unaffected: it + always emits per-target rows regardless of the flag. Unknown aliases opted + into target capability are assumed to accept repeated `--target`; if they do + not, Cargo's own error surfaces. V1 intentionally has one target capability + bit, not a separate "accepts repeated --target" capability; the built-in `run` + exception is hardcoded from Cargo's behavior. +- **Fallbacks and no-ops.** Aggregate mode falls back to serial when `run` is the + selected subcommand, when pruned summaries are enabled, or when the effective + run has only one target. For `cargo fc matrix`, `--aggregate-targets` has no + effect because matrix output is always per target. When the user explicitly + passed `--aggregate-targets` and it has no effect or falls back, emit one short + note explaining why. +- **Determinism.** Iterate packages in plan order, combos in sorted order, and + targets within an invocation in target-plan order. `--fail-fast` stops at the + first failing invocation. `--dedupe` uses the same global `seen` set across + invocations. + +### Architecture impact + +These decisions remove every concurrency abstraction the earlier drafts carried +under the worker-pool assumption: no thread/worker pool, no `OutputSink`, no +buffered capture or ordered replay, no per-worker job budgeting, no +`max_concurrent_targets`/`--max-concurrent-targets`, and no cross-worker +exit-code coordination. Preassigned progress indices also go away — the existing +up-front-total-plus-runtime-counter scheme is enough. What remains is one extra +boolean flag, one `TargetExecutionMode` enum, a `SummaryTarget` generalization, +and a per-package canonical combo-to-targets transpose in the executor; +planning, config resolution, and feature generation are identical for both +modes. The summary/output generalization is the main real cost of aggregate +mode; do not hide that complexity in the executor. + +## Runner API Changes + +Keep the existing single-target functions for compatibility: + +```rust +pub fn print_feature_matrix_for_target(...) +pub fn run_cargo_command_for_target(...) +``` + +Draw the new public boundary at `ExecutionPlan`, not `TargetPlan`. Planning +(target selection, per-target config resolution, feature generation/pruning) +needs the cached package configs and the `CfgEvaluator`; execution does not. +Building the execution plans in the caller keeps config loading in one place (no +duplicated deprecation warnings, no re-parsing) and lets the executor be pure: + +```rust +// Planning: needs cached configs (via PlannedPackage::config) + evaluator. +pub fn build_execution_plans( + target_plans: &[TargetPlan<'_>], + options: &Options, + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result>> + +// Execution: consumes prebuilt plans; needs neither config nor evaluator. +// `mode` selects serial per-target vs aggregate; both are single-threaded. +pub fn run_execution_plans( + plans: &[ExecutionPlan<'_>], + cargo_args: Vec<&str>, + options: &Options, + mode: TargetExecutionMode, +) -> eyre::Result + +pub fn print_matrix_for_execution_plans( + plans: &[ExecutionPlan<'_>], + options: &MatrixOptions, +) -> eyre::Result +``` + +The exact names can differ, but keep three properties: target planning is +separate from feature-combination planning; base configs are loaded once and +flow through `PlannedPackage::config`; and the executor takes already-resolved +plans, so it needs no `CfgEvaluator` and re-reads no manifests. This is also why +the earlier API sketch of `run_cargo_command_for_targets(plans: &[TargetPlan], +…, evaluator)` was dropped: passing `TargetPlan`s into the runner would force it +to re-resolve configs and re-trigger deprecation warnings. + +## Execution Plan Model + +To keep execution simple and deterministic, introduce a second plan after target +planning: + +```rust +pub struct ExecutionPlan<'a> { + pub target: TargetTriple, + pub package_plans: Vec>, + pub show_pruned: bool, +} + +pub struct PackageExecutionPlan<'a> { + pub package: &'a cargo_metadata::Package, + pub target: EffectiveTarget, + pub combinations: Vec>, + pub pruned: Vec, +} +``` + +The exact container shape can differ. A flat ordered list of resolved +`PackageTargetExecutionPlan` units is also acceptable if it makes the executor +simpler. The invariant matters more than the struct names: after +`build_execution_plans`, execution owns a deterministic sequence of resolved +`(package, target, combinations, pruned)` units, with no need to load configs or +evaluate cfg again. + +Build `ExecutionPlan`s serially before running Cargo (this is +`build_execution_plans` from "Runner API Changes"): + +1. For each `TargetPlan`, resolve each package assignment's target-specific + config from the cached `PlannedPackage::config` (never re-read the manifest). +2. Generate and prune feature combinations. +3. Store owned feature strings so execution does not borrow from temporary + configs. +4. Compute the total invocation count up front (serial: total combos across all + `(package, target)`; aggregate: number of distinct `(package, combo)` + invocations). The progress counter increments at runtime in iteration order. + +This mirrors the existing `plan_feature_combinations` function but lifts it from +"one target" to "all planned targets". The runner should then execute an +already-built plan. + +Both execution modes consume these same plans. Serial iterates them directly +(`(package, target)` then combo). Aggregate transposes them per package into +`canonical combo -> targets` and emits one invocation per `(package, combo)`; no +separate plan type is required. + +Why this matters: + +- target override resolution stays deterministic and easy to test, +- the executor consumes owned plans and needs neither `CfgEvaluator` nor package + configs, so it does no metadata parsing and is trivial to test, +- fail-fast and progress counts can be defined before execution starts, +- both the serial and aggregate executors consume the same plan, differing only + in how many `--target` flags each invocation carries. + +## Execution Strategy + +Both modes execute the same `ExecutionPlan` list on a single thread: + +```rust +pub enum TargetExecutionMode { + /// Default: one invocation per (package, target, combo). + SerialPerTarget, + /// `--aggregate-targets`: one invocation per (package, combo), all targets. + Aggregate, +} +``` + +Both: + +- stream live Cargo output (no buffering or replay), +- use one shared diagnostic dedupe set, +- honor `--fail-fast` by stopping at the first failing invocation, +- report cargo-fc-controlled invocation and summary order deterministically. + +In aggregate mode, Cargo controls diagnostic ordering inside the one +multi-target invocation. cargo-fc must not claim per-target diagnostic +attribution or ordering there. + +The mode only changes how targets map onto invocations and how summary entries +are keyed; planning, config resolution, and feature generation are identical. + +## Implementation Milestones + +### Milestone 1: Config and Target Planning + +- Add `WorkspaceConfig::targets`. +- Add `WorkspaceConfig::target` with `WorkspaceTargetOverride`. +- Add `Config::targets: Option>`. +- Add workspace `subcommands..targets` for alias target opt-in. +- Add target-source domain types. +- Add raw cargo subcommand token extraction for capability lookup. +- Add target planning module with narrow test adapters. +- Split workspace package discovery from workspace package exclusion so + exclusions can be applied per target. +- Cache selected package configs once and reuse them for planning and execution + planning. +- Evaluate workspace target override cfg keys through `CfgEvaluator`. +- Parse forwarded Cargo `--target` only before `--`. +- Detect raw configured target lists before capability filtering so skipped + target warnings have a reliable data source. +- Add tests for serde behavior: + - absent package targets inherit workspace targets, + - empty package targets opt out to fallback single target, + - package targets override workspace targets, + - duplicate targets are removed while preserving first occurrence, + - workspace target overrides patch `exclude_packages`. +- Add tests for target precedence, target plan ordering, and source tracking. +- Add a regression test where the same target triple is reached through + different sources for different packages, and verify injection decisions are + preserved per package assignment. +- Add tests for non-root workspace-only metadata warnings for `targets`, + workspace target overrides, and `subcommands`. + +### Milestone 2: Multi-Target Matrix + +- Add `print_matrix_for_execution_plans`. +- Build `ExecutionPlan`s for matrix output instead of recomputing target + selection inside the matrix function. +- Include `target` in every matrix row. +- Warn if user matrix metadata already contains the reserved `target` key. +- Ensure target-specific overrides are resolved per package-target pair. +- Add integration tests for: + - workspace targets, + - package-level targets, + - target-specific workspace `exclude_packages`, + - `--target` overriding configured lists, + - target override sections changing feature rows for matching targets. + +### Milestone 3: Serial Multi-Target Execution + +- Add the execution-plan executor (`run_execution_plans`) for serial per-target + mode. +- Build `ExecutionPlan`s before execution. +- Inject `--target ` for configured targets when the command has the + `targets` capability. +- Add `SummaryTarget`-style target display context to `Summary`. +- Add target to command/progress display. +- Add target-aware summary output. +- Count summaries by `(package, target, features)`. +- Scope pruned summary attachment by `(package, target)`. +- Compute invocation totals up front; increment the progress counter at runtime + in plan order. +- Extend `--dedupe` across serial target plans. +- Keep `--fail-fast` serial and deterministic. +- Preserve existing `--diagnostics-only` and `--dedupe` eligibility for + `test`, `run`, and aliases. +- Aggregate exit codes deterministically by returning the first failure in + execution-plan order after non-fail-fast execution completes. + +This milestone delivers the core serial feature. + +### Milestone 4: Aggregate Execution Mode + +- Add the `--aggregate-targets` flag (boolean, drained before forwarding). +- Add `TargetExecutionMode` and route `run_execution_plans` by mode. +- In aggregate mode, group each package's executed combinations by canonical + combo key and emit one Cargo invocation per `(package, combo)` with a + `--target` per target that has that combo. +- Key summaries by `(package, combo, target-group)` and show `targets = [...]`. + Singleton target groups should keep the normal `target = ...` summary shape. +- Fall back to serial per-target for `run` (Cargo rejects multiple `--target`), + with a one-line note. +- Fall back to serial per-target when pruned summaries are enabled + (`--show-pruned` or resolved config `show_pruned = true`), with a one-line + note. +- Treat `--aggregate-targets` as a no-op for matrix and single-target runs, with + a one-line note when the user explicitly passed the flag. +- Document and implement group-level count semantics: aggregate warning/error + counts are for the Cargo invocation's target group and may differ from serial + per-target counts. Recommend pairing `--aggregate-targets` with `--dedupe` for + diagnostics-heavy output. +- Reuse the existing per-invocation and global dedupe; add no new output path. + +### Milestone 5: Docs + +- Leave `README.md` unchanged until the implementation is complete. +- Once the full feature lands, update `README.md` with the final shipped design + for configured targets, target capability, and alias target opt-in. +- Document workspace targets. +- Document target-specific workspace `exclude_packages`. +- Document package-level targets and package opt-out with `targets = []`. +- Document built-in target capability and alias opt-in. +- Document the two execution modes: serial per-target (default, exact + attribution) and `--aggregate-targets` (one multi-`--target` invocation per + combo, group-level attribution, faster on many cores), why there is no worker + pool, and that `--aggregate-targets` falls back to serial for `run`; include a + short summary of the measurements. +- Document that `--aggregate-targets` falls back to serial when pruned summaries + are enabled, is a no-op for matrix/single-target runs, and reports group-level + counts that may differ from serial per-target counts. Recommend `--dedupe` + when combining aggregate mode with diagnostics-only output. +- Document that the workspace `targets` list also applies to `test`/`run`, that + foreign-target `test`/`run` typically fail, and that `--target` narrows a run. +- Document rustup target prerequisites. +- Document that configured target lists intentionally take precedence over + `CARGO_BUILD_TARGET`; use explicit `--target` to select one target. +- Document the `cargo fc matrix` `target` key schema change and reserved-key + warning. +- Update GitHub Actions docs to show a single `cargo fc clippy` or + `cargo fc check` invocation. + +### No further parallelism milestone + +Aggregate mode (the only candidate that survived the measurements in "Target +Execution Modes") is part of v1 as Milestone 4. A worker pool of concurrent Cargo +processes was measured and rejected, so no parallelism work remains. + +## Testing Checklist + +- Unit tests: + - target flag parsing recognizes `--target x` and `--target=x` before `--`, + and ignores `--target` after `--`, + - target precedence, + - target list dedupe with stable order, + - target plan order preserves workspace target order before package-only + targets, + - same-triple package assignments can keep different target sources, + - target-specific workspace `exclude_packages` removes packages only from + matching target plans, + - target-specific workspace `exclude_packages` applies to explicit + single-target invocations, + - workspace/package target resolution, + - built-in `clippy` has target capability, + - unknown `lint` lacks target capability unless configured, + - configured-target warning is emitted once per invocation and is based on raw + config state before capability filtering, + - package configs are loaded once and deprecation warnings are not duplicated, + - summary formatting includes target, + - serial summary counts key by package, target, and features, + - serial pruned summaries are attached only within the same package and target, + - aggregate summaries key by package, canonical combo, and target group, + - aggregate mode falls back to serial when pruned summaries are enabled, + - invocation totals are computed up front and the progress counter increments + in plan order, + - implicit single-host runs do not print `target = ...` in headers or + summaries, + - per-combination diagnostics header includes the target when targets are + configured, explicitly selected, or more than one is planned, + - matrix rows include target, + - matrix emits the reserved-`target`-key warning when package matrix metadata + already defines `target`, + - aggregate mode batches targets sharing a combo into one invocation with a + `--target` per target, + - aggregate mode groups by canonical feature-combination key, not display + text, + - aggregate mode keeps targets with divergent feature matrices in separate + per-combo invocations, + - `--aggregate-targets` falls back to serial for `run`, + - `--aggregate-targets` is a no-op for single-target execution and matrix. + +- Integration tests: + - no configured targets preserves existing behavior, + - workspace targets multiply matrix rows, + - package targets override workspace targets, + - workspace target overrides change package participation per target, + - workspace target overrides apply when `--target` selects a single matching + target, + - package `targets = []` opts out of workspace targets, + - explicit `--target` ignores configured target lists, + - target override sections still apply correctly, + - configured target missing from rustup produces a clear failure, + - `--diagnostics-only`/`--dedupe` behavior for `test`, `run`, and aliases is + not regressed, + - matrix output for a workspace with no configured targets still includes + `target` = host on every row (additive schema change), + - `--aggregate-targets` does not change matrix output, + - serial and aggregate modes produce the same pass/fail verdict for a clean and + a failing combination across two targets, + - aggregate diagnostics counts are group-level and may differ from serial + counts; with `--dedupe`, repeated common diagnostics are rendered once for + the group, + - aggregate summaries show `targets = [...]` for batched multi-target + invocations and `target = ...` for singleton groups. + +## Readiness Criteria + +The v1 feature is complete when configured targets, package-level targets, +target-specific workspace exclusions, target capability, the serial per-target +and aggregate (`--aggregate-targets`) execution modes, matrix output, tests, and +final README documentation all land together. + +Closed decisions: + +- cargo-fc is single-threaded and never spawns concurrent Cargo processes. The + only execution flag is `--aggregate-targets` (default off); there is no + `--max-concurrent-targets` flag, no `max_concurrent_targets` config key, and no + worker/thread pool. +- Target plans are deduplicated by target triple, while package-target + assignments carry `TargetSource` for per-package injection decisions. +- Workspace target overrides may patch only `exclude_packages`. +- Command target capability is workspace-level policy for aliases. Built-in + `clippy` is known; literal `lint` is unknown unless configured. +- Unknown aliases retain legacy best-effort feature selection, but configured + targets require explicit target capability. +- Existing `--diagnostics-only`/`--dedupe` eligibility is preserved in v1. +- Execution model is resolved: two single-threaded modes — serial per-target + (default, exact attribution) and aggregate `--aggregate-targets` (one + multi-`--target` invocation per combo, group-level attribution for batched + multi-target groups, group-level counts, serial fallback for `run` and pruned + summaries). A worker pool of concurrent Cargo processes was measured and + rejected (no gain on a shared `target/`, a net loss on small runners). See + "Target Execution Modes." +- Leave `README.md` unchanged until the feature is fully implemented, then + document the final shipped design. + +## Error Messages + +Missing Rust target: + +```text +target `x86_64-pc-windows-msvc` is not installed +hint: run `rustup target add x86_64-pc-windows-msvc` +``` + +Configured targets with a command that lacks target capability: + +```text +warning: not passing configured targets to cargo alias `lint` because it has no targets capability +hint: add [workspace.metadata.cargo-fc.subcommands.lint] targets = true if this alias accepts --target +``` + +`--aggregate-targets` with `run` (falls back to serial): + +```text +note: --aggregate-targets does not apply to `run` (cargo runs one target at a time); running targets serially +``` + +`--aggregate-targets` with pruned summaries enabled: + +```text +note: --aggregate-targets is disabled because pruned summaries are target-specific; running targets serially +``` + +`--aggregate-targets` with no effect: + +```text +note: --aggregate-targets has no effect for a single target; running normally +note: --aggregate-targets has no effect for matrix output; matrix rows are always per target +``` From af32c27e20fbb1e6d57374d5a648ee0eb60d0341 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 20:15:16 +0200 Subject: [PATCH 02/13] feat: add per-target feature combination axis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a target axis so a workspace can declare target triples checked by default, turning a run into `selected packages × effective targets × feature combinations`. A plain `cargo fc check` now exercises the same target cfg views as CI. Config: - `Config.targets` (None=inherit, []=opt-out, list=override), cleared in config resolution so feature generation never reads it. - `WorkspaceConfig.targets`, target-specific `target.'cfg(...)'` `exclude_packages` overrides, and a `subcommands` target-capability table. Planning (new `target_plan` module): - precedence `--target` > package > workspace > `CARGO_BUILD_TARGET` > host, with stable dedupe/ordering and a per-assignment `TargetSource`. - per-target workspace `exclude_packages` resolution. - target capability resolution: built-in subcommands are known; unknown aliases need explicit `subcommands..targets = true`. - split workspace candidate discovery from exclusion so excludes apply per target; cache package configs once to avoid duplicate deprecation warnings. Execution: - precomputed `ExecutionPlan`s drive both modes. - serial per-target (default): one invocation per (package, target, combo) with exact attribution, `--target` injected for configured sources, `--dedupe` shared across plans. - opt-in `--aggregate-targets`: one invocation per (package, combo) batching shared targets as repeated `--target`, group-level attribution; falls back to serial for `run`, pruned summaries, and single-target runs. - target-aware headers/summaries (`SummaryTarget`); implicit single-host output is unchanged. Matrix: every `cargo fc matrix` row carries `target`; the built-in key wins over user matrix metadata with a compatibility warning. Docs: document configured targets, capability/alias opt-in, the two execution modes, and the matrix schema change in README and GitHub Actions docs. Surface rustc's reason at the cfg-eval boundary. --- README.md | 165 ++++++++ docs/GITHUB_ACTIONS.md | 36 ++ src/cfg_eval.rs | 23 +- src/cli.rs | 248 ++++++++++- src/config.rs | 59 +++ src/config/resolve.rs | 6 + src/lib.rs | 305 ++++++++++++-- src/runner.rs | 903 ++++++++++++++++++++++++++++++++++------- src/target.rs | 271 +++++++++++-- src/target_plan.rs | 891 ++++++++++++++++++++++++++++++++++++++++ src/workspace.rs | 192 +++++---- tests/per_target.rs | 498 +++++++++++++++++++++++ 12 files changed, 3288 insertions(+), 309 deletions(-) create mode 100644 src/target_plan.rs create mode 100644 tests/per_target.rs diff --git a/README.md b/README.md index 19eb62a..2896937 100644 --- a/README.md +++ b/README.md @@ -115,6 +115,11 @@ OPTIONS: --errors-only Allow all warnings, show errors only (-Awarnings) --pedantic Treat warnings like errors in summary and when using --fail-fast + --no-prune-implied Disable pruning of redundant feature combinations + --show-pruned Show pruned feature combinations in the summary + --aggregate-targets Batch a combination's configured targets into one + Cargo invocation (faster on many cores; group-level + attribution). See "Configured targets" below. ``` ### Configuration @@ -265,6 +270,166 @@ combinations that include `tokio` or `serde`, you can list them explicitly in --- +### Configured targets + +By default `cargo fc` runs for a single effective target (the same one Cargo +would pick: `--target`, then `CARGO_BUILD_TARGET`, then the host). You can +instead declare a list of target triples to check by default, turning the run +into a full matrix of + +```text +selected packages × effective targets × feature combinations +``` + +so that a plain local `cargo fc check` exercises exactly the target cfg views +that CI exercises. + +Declare workspace-wide targets in the workspace `Cargo.toml`: + +```toml +[workspace.metadata.cargo-fc] +targets = [ + "x86_64-unknown-linux-gnu", + "x86_64-pc-windows-msvc", + "aarch64-apple-darwin", +] +``` + +Individual packages can override the workspace list, or opt out of it: + +```toml +[package.metadata.cargo-fc] +# Run only this package on wasm (overrides the workspace list, does not merge). +targets = ["wasm32-unknown-unknown"] + +# Or opt out of configured targets entirely and use the single effective target: +# targets = [] +``` + +- **missing key** — inherit the workspace target list, +- **`targets = []`** — opt out of the workspace list and use the single + effective target (`CARGO_BUILD_TARGET`, then host), +- **`targets = ["…"]`** — this package's own target list (overrides, not + merges with, the workspace list). + +`targets` only selects which targets are visited. The +[`target.'cfg(...)'`](#target-specific-configuration) overrides below still +shape the feature matrix for each concrete target. + +#### Precedence + +When the selected command supports targets, each package's targets are resolved as: + +1. an explicit Cargo `--target ` (wins globally for that run), +2. the package's `targets`, +3. the workspace `targets`, +4. `CARGO_BUILD_TARGET`, +5. the host target. + +> [!IMPORTANT] +> Configured target lists intentionally take precedence over +> `CARGO_BUILD_TARGET` — repository config is the declarative matrix and should +> not be silently collapsed by a developer's ambient environment. This differs +> from Cargo's own `[build].target` precedence. To run a single target for one +> invocation, pass an explicit `--target `, which overrides all +> configured lists. + +#### Which commands receive configured targets + +Configured targets are applied only to commands that accept Cargo's `--target` +flag. Built-in subcommands cargo-fc recognizes — `check`, `clippy`, `build`, +`doc`, `test`, `run` (and `cargo fc matrix`) — get this capability +automatically. + +Unknown aliases and custom subcommands do **not** receive configured targets +unless you opt them in. cargo-fc will not guess that `cargo lint` means +`cargo clippy`; instead, declare it: + +```toml +[workspace.metadata.cargo-fc.subcommands.lint] +targets = true +``` + +If configured targets exist but the selected command lacks this capability, +cargo-fc warns once and falls back to the single effective target. + +> [!WARNING] +> The `targets` list is shared by all target-capable commands. It is motivated +> by `check`/`clippy` (which only need the target's `rustc`), but it also +> applies to `build` (needs a linker), and to `test`/`run` (which execute and +> therefore usually fail for foreign targets). Narrow a single run with an +> explicit `--target ` when needed. Missing targets surface Cargo's own +> `rustup target add ` hint; cargo-fc does not install targets for you. + +#### Per-target workspace package selection + +Workspace package exclusions can vary by target, using the same `cfg(...)` +selectors and patch semantics as the feature overrides: + +```toml +[workspace.metadata.cargo-fc] +targets = ["x86_64-unknown-linux-gnu", "wasm32-unknown-unknown"] + +[workspace.metadata.cargo-fc.target.'cfg(target_arch = "wasm32")'] +exclude_packages = { add = ["native-cli"] } + +[workspace.metadata.cargo-fc.target.'cfg(target_os = "linux")'] +exclude_packages = { add = ["wasm-app"] } +``` + +Workspace target overrides may patch `exclude_packages` only, and they apply to +every concrete effective target — including single-target runs selected by +`--target`, `CARGO_BUILD_TARGET`, or the host. + +#### Matrix output + +Every `cargo fc matrix` row now includes a `target` field: + +```json +{ "name": "my-crate", "target": "x86_64-pc-windows-msvc", "features": "serde,cli" } +``` + +`target` is a reserved built-in key: if your `matrix` metadata already defines +`target`, the built-in value wins and cargo-fc warns. (This is an additive +schema change — runs with no configured targets still emit `target` = the host +triple on every row.) + +#### Execution modes + +cargo-fc is single-threaded and never spawns concurrent Cargo processes; it +relies on Cargo's own scheduler to use your cores. There are two modes: + +- **serial per-target (default)** — one Cargo invocation per + `(package, target, combination)`. Output stays live and every PASS/FAIL, + diagnostic, and dedupe note is attributed to exactly one target. +- **`--aggregate-targets`** — one Cargo invocation per `(package, combination)` + that passes every target sharing that combination as repeated `--target` + flags, letting Cargo overlap their build graphs. Faster on many-core machines, + but results are reported per target *group* (`targets = [a, b, …]`) rather + than per target. + +A worker pool of concurrent Cargo processes was measured and rejected: with a +shared `target/` directory, Cargo serializes on its build-directory lock, so +concurrent `--target` builds give no speedup; per-target `CARGO_TARGET_DIR` +workers do parallelize but recompile shared host artifacts per target (a small +win on many cores, ~28% slower on 2 cores, plus doubled disk). A single +multi-target invocation (aggregate mode) is the only approach that is faster on +many cores and never slower on small CI runners. + +`--aggregate-targets` falls back to serial per-target when: + +- the subcommand is `run` (Cargo rejects multiple `--target` for `run`), +- pruned summaries are enabled (`--show-pruned` or `show_pruned` in config) — + pruning is target-specific, +- only one target is effectively planned, + +and it has no effect on `cargo fc matrix` (rows are always per target). In each +case cargo-fc prints a short note. Aggregate warning/error counts are for the +whole target group and may differ from serial per-target counts; pair +`--aggregate-targets` with `--dedupe` for the cleanest diagnostics output. + +--- + ### Target-specific configuration You can override configuration for specific targets using Cargo-style `cfg(...)` expressions. diff --git a/docs/GITHUB_ACTIONS.md b/docs/GITHUB_ACTIONS.md index 45b54eb..389cdab 100644 --- a/docs/GITHUB_ACTIONS.md +++ b/docs/GITHUB_ACTIONS.md @@ -69,4 +69,40 @@ jobs: Of course you can also apply the same approach for your `test.yaml` or `lint.yaml` workflows! Per job, up to 256 feature sets can be processed in parallel. +### Configured targets + +If you declare [configured targets](../README.md#configured-targets) in your +`Cargo.toml`, every `cargo fc matrix` row also carries a `target` field, so you +can fan out the GitHub Actions matrix over targets too: + +```yaml +strategy: + fail-fast: false + matrix: + package: ${{ fromJson(needs.feature-matrix.outputs.matrix) }} +steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + targets: ${{ matrix.package.target }} + - run: >- + cargo check + --package "${{ matrix.package.name }}" + --features "${{ matrix.package.features }}" + --target "${{ matrix.package.target }}" +``` + +Alternatively, for linting/checking where you do not need a separate CI job per +combination, a **single** invocation iterates every configured target and +feature combination for you (no GitHub Actions matrix required): + +```yaml +- uses: actions/checkout@v4 +- uses: romnn/cargo-feature-combinations@main +- run: cargo fc clippy # or: cargo fc check +``` + +Add `--aggregate-targets` to batch each combination's targets into one Cargo +invocation for extra throughput on many-core runners. + diff --git a/src/cfg_eval.rs b/src/cfg_eval.rs index 4a31e55..274a6d2 100644 --- a/src/cfg_eval.rs +++ b/src/cfg_eval.rs @@ -81,11 +81,24 @@ impl RustcCfgEvaluator { })?; if !output.status.success() { - eyre::bail!( - "rustc --print cfg --target {} failed with exit code {:?}", - key, - output.status.code() - ); + // Classify the failure at this adapter boundary: surface rustc's + // own reason, and add a rustup hint when the triple looks like a + // valid-but-not-installed target. (Note: `rustc --print cfg` + // succeeds for known targets even when their std is missing, so + // in practice this path is reached for unknown/invalid triples; + // the rustup hint is only added when rustc says so.) + use std::fmt::Write as _; + let stderr = String::from_utf8_lossy(&output.stderr); + let stderr = stderr.trim(); + let mut message = format!("failed to read cfg for target `{key}`"); + if !stderr.is_empty() { + message.push('\n'); + message.push_str(stderr); + } + if stderr.contains("not installed") || stderr.contains("rustup target add") { + let _ = write!(message, "\nhint: run `rustup target add {key}`"); + } + eyre::bail!(message); } let stdout = String::from_utf8_lossy(&output.stdout); diff --git a/src/cli.rs b/src/cli.rs index ad0ecb8..49e77b5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -76,6 +76,12 @@ pub struct Options { /// /// Set by `--show-pruned`. pub show_pruned: bool, + /// Whether to use aggregate target execution mode. + /// + /// Set by `--aggregate-targets`. When enabled, one Cargo invocation per + /// `(package, combo)` carries every target sharing that combo as repeated + /// `--target` flags. The default (flag absent) is serial per-target. + pub aggregate_targets: bool, } /// Helper trait to provide simple argument parsing over `Vec`. @@ -137,18 +143,33 @@ pub(crate) enum CargoSubcommand { /// Determine the cargo subcommand implied by the argument list. pub(crate) fn cargo_subcommand(args: &[impl AsRef]) -> CargoSubcommand { - fn subcommand_from_token(arg: &str) -> CargoSubcommand { - match arg { - "build" | "b" => CargoSubcommand::Build, - "check" | "c" => CargoSubcommand::Check, - "clippy" => CargoSubcommand::Lint, - "test" | "t" => CargoSubcommand::Test, - "doc" | "d" => CargoSubcommand::Doc, - "run" | "r" => CargoSubcommand::Run, - _ => CargoSubcommand::Other, - } + match cargo_subcommand_token(args) { + Some(token) => subcommand_from_token(&token), + None => CargoSubcommand::Other, + } +} + +fn subcommand_from_token(arg: &str) -> CargoSubcommand { + match arg { + "build" | "b" => CargoSubcommand::Build, + "check" | "c" => CargoSubcommand::Check, + "clippy" => CargoSubcommand::Lint, + "test" | "t" => CargoSubcommand::Test, + "doc" | "d" => CargoSubcommand::Doc, + "run" | "r" => CargoSubcommand::Run, + _ => CargoSubcommand::Other, } +} +/// Extract the raw cargo subcommand token from the argument list. +/// +/// Unlike [`cargo_subcommand`], this preserves the literal token (e.g. `lint`, +/// `clippy`, `c`) so the command target-capability registry can reason about +/// aliases that the [`CargoSubcommand`] enum collapses to `Other`. +/// +/// Returns `None` when no subcommand token is present (e.g. an unknown leading +/// flag or an early `--`). +pub(crate) fn cargo_subcommand_token(args: &[impl AsRef]) -> Option { fn is_no_value_flag(arg: &str) -> bool { matches!( arg, @@ -184,7 +205,7 @@ pub(crate) fn cargo_subcommand(args: &[impl AsRef]) -> CargoSubcommand { continue; } if arg == "--" { - return CargoSubcommand::Other; + return None; } if arg.starts_with('+') { continue; @@ -201,17 +222,129 @@ pub(crate) fn cargo_subcommand(args: &[impl AsRef]) -> CargoSubcommand { } if arg.starts_with("--") { - return CargoSubcommand::Other; + return None; } if arg.starts_with('-') { - return CargoSubcommand::Other; + return None; + } + + return Some(arg.to_string()); + } + + None +} + +/// Built-in cargo subcommand tokens cargo-fc knows accept Cargo's `--target`. +/// +/// This is the initial target-capability registry. It tracks the command +/// tokens cargo-fc already recognizes today (`build`, `check`, `clippy`, +/// `test`, `doc`, `run`) plus their short aliases. `matrix` is handled +/// separately because it is not a forwarded cargo command. +fn builtin_target_capability(token: &str) -> Option { + match token { + "build" | "b" | "check" | "c" | "clippy" | "test" | "t" | "doc" | "d" | "run" | "r" => { + Some(true) + } + _ => None, + } +} + +/// Where a command's target capability was decided. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CommandCapabilitySource { + /// A known built-in cargo subcommand. + BuiltIn, + /// An alias/custom command opted in via workspace config. + WorkspaceConfig, + /// An unknown command with no built-in entry and no workspace opt-in. + Unknown, +} + +/// Whether configured targets / `--target` injection apply to a command. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CapabilityDecision { + /// cargo-fc may expand configured target lists and inject `--target`. + Allowed, + /// Configured target lists are ignored for this command. + Denied, +} + +impl CapabilityDecision { + /// Whether the decision allows configured targets. + #[must_use] + pub fn is_allowed(self) -> bool { + matches!(self, Self::Allowed) + } +} + +/// Resolved target-capability policy for the selected command. +/// +/// The runner consumes one resolved policy object instead of repeatedly asking +/// whether a subcommand is known. +#[derive(Debug, Clone)] +pub struct ResolvedCommandTargetPolicy { + /// The detected command token (e.g. `clippy`, `lint`), or `matrix`. + pub command_name: String, + /// Where the capability decision came from. + pub source: CommandCapabilitySource, + /// Whether configured targets apply to this command. + pub targets: CapabilityDecision, +} + +/// Resolve the target-capability policy for a forwarded cargo command token. +/// +/// Built-in commands get their capability from code and ignore workspace +/// config. If a workspace `subcommands.` entry shadows a built-in +/// command, it is ignored with a warning. Unknown tokens are denied unless the +/// workspace opts them in with `targets = true`. +#[must_use] +pub fn resolve_command_target_policy( + token: Option<&str>, + subcommands: &std::collections::BTreeMap, +) -> ResolvedCommandTargetPolicy { + let Some(token) = token else { + return ResolvedCommandTargetPolicy { + command_name: String::new(), + source: CommandCapabilitySource::Unknown, + targets: CapabilityDecision::Denied, + }; + }; + + if let Some(allowed) = builtin_target_capability(token) { + if subcommands.contains_key(token) { + crate::print_warning!( + "ignoring [workspace.metadata.cargo-fc.subcommands.{token}]: `{token}` is a built-in cargo subcommand whose target capability is provided by cargo-fc" + ); } + return ResolvedCommandTargetPolicy { + command_name: token.to_string(), + source: CommandCapabilitySource::BuiltIn, + targets: if allowed { + CapabilityDecision::Allowed + } else { + CapabilityDecision::Denied + }, + }; + } - return subcommand_from_token(arg); + if let Some(capability) = subcommands.get(token) { + return ResolvedCommandTargetPolicy { + command_name: token.to_string(), + source: CommandCapabilitySource::WorkspaceConfig, + targets: if capability.targets { + CapabilityDecision::Allowed + } else { + CapabilityDecision::Denied + }, + }; } - CargoSubcommand::Other + ResolvedCommandTargetPolicy { + command_name: token.to_string(), + source: CommandCapabilitySource::Unknown, + targets: CapabilityDecision::Denied, + } } static VALID_BOOLS: [&str; 4] = ["yes", "true", "y", "t"]; @@ -246,6 +379,11 @@ OPTIONS: --no-prune-implied Disable automatic pruning of redundant feature combinations implied by other features --show-pruned Show pruned feature combinations in the summary + --aggregate-targets Batch each combination's configured targets into a + single Cargo invocation (one `--target` per target) + instead of one invocation per target. Faster on + many cores; reports results per target group. Falls + back to serial for `run` and pruned summaries. Feature sets can be configured in your Cargo.toml configuration. The following metadata key aliases are all supported: @@ -483,6 +621,7 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec)> { drain_flag("--fail-fast", &mut options.fail_fast); drain_flag("--no-prune-implied", &mut options.no_prune_implied); drain_flag("--show-pruned", &mut options.show_pruned); + drain_flag("--aggregate-targets", &mut options.aggregate_targets); // --dedupe implies --diagnostics-only for flag in ["--dedupe", "--dedup"] { @@ -516,8 +655,13 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec)> { #[cfg(test)] mod test { - use super::{CargoSubcommand, cargo_subcommand}; + use super::{ + CapabilityDecision, CargoSubcommand, CommandCapabilitySource, cargo_subcommand, + cargo_subcommand_token, resolve_command_target_policy, + }; + use crate::config::CommandTargetCapability; use similar_asserts::assert_eq as sim_assert_eq; + use std::collections::BTreeMap; #[test] fn cargo_subcommand_detects_build_and_short_build() { @@ -594,4 +738,76 @@ mod test { sim_assert_eq!(cargo_subcommand(&["lint"]), CargoSubcommand::Other); sim_assert_eq!(cargo_subcommand(&["lint", "build"]), CargoSubcommand::Other); } + + #[test] + fn cargo_subcommand_token_preserves_literal_token() { + sim_assert_eq!( + cargo_subcommand_token(&["clippy"]), + Some("clippy".to_string()) + ); + sim_assert_eq!(cargo_subcommand_token(&["lint"]), Some("lint".to_string())); + sim_assert_eq!(cargo_subcommand_token(&["c"]), Some("c".to_string())); + sim_assert_eq!( + cargo_subcommand_token(&["+nightly", "--frozen", "lint", "build"]), + Some("lint".to_string()) + ); + } + + #[test] + fn cargo_subcommand_token_none_for_missing_command() { + let empty: [&str; 0] = []; + sim_assert_eq!(cargo_subcommand_token(&empty), None); + sim_assert_eq!(cargo_subcommand_token(&["--mystery-flag"]), None); + sim_assert_eq!(cargo_subcommand_token(&["--"]), None); + } + + #[test] + fn builtin_clippy_has_target_capability() { + let subcommands = BTreeMap::new(); + let policy = resolve_command_target_policy(Some("clippy"), &subcommands); + sim_assert_eq!(policy.source, CommandCapabilitySource::BuiltIn); + sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + } + + #[test] + fn unknown_lint_lacks_target_capability_unless_configured() { + let empty = BTreeMap::new(); + let policy = resolve_command_target_policy(Some("lint"), &empty); + sim_assert_eq!(policy.source, CommandCapabilitySource::Unknown); + sim_assert_eq!(policy.targets, CapabilityDecision::Denied); + + let mut configured = BTreeMap::new(); + configured.insert( + "lint".to_string(), + CommandTargetCapability { targets: true }, + ); + let policy = resolve_command_target_policy(Some("lint"), &configured); + sim_assert_eq!(policy.source, CommandCapabilitySource::WorkspaceConfig); + sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + } + + #[test] + fn configured_alias_with_targets_false_is_denied() { + let mut configured = BTreeMap::new(); + configured.insert( + "lint".to_string(), + CommandTargetCapability { targets: false }, + ); + let policy = resolve_command_target_policy(Some("lint"), &configured); + sim_assert_eq!(policy.source, CommandCapabilitySource::WorkspaceConfig); + sim_assert_eq!(policy.targets, CapabilityDecision::Denied); + } + + #[test] + fn builtin_capability_ignores_workspace_shadowing_entry() { + let mut configured = BTreeMap::new(); + configured.insert( + "check".to_string(), + CommandTargetCapability { targets: false }, + ); + // A workspace entry shadowing a built-in must not flip its capability. + let policy = resolve_command_target_policy(Some("check"), &configured); + sim_assert_eq!(policy.source, CommandCapabilitySource::BuiltIn); + sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + } } diff --git a/src/config.rs b/src/config.rs index 2587a94..a46dd51 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,6 +19,20 @@ fn default_true() -> bool { /// `[workspace.metadata.cargo-fc]` instead. #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Config { + /// Package-level target triples to check by default. + /// + /// This is a target *selection* field, not a feature-matrix field: + /// + /// - `None` (key absent): inherit the workspace target list. + /// - `Some([])`: explicit opt-out of the workspace target list; use the + /// single effective target (`CARGO_BUILD_TARGET` or host) instead. + /// - `Some([..])`: this package's own target list, overriding the workspace + /// list. + /// + /// `targets` is never read by feature-combination generation. Target + /// override sections (`target.'cfg(...)'`) must not change it. + #[serde(default)] + pub targets: Option>, /// Feature sets that must be tested in isolation. #[serde(default)] pub isolated_feature_sets: Vec>, @@ -95,6 +109,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { + targets: None, isolated_feature_sets: Vec::new(), exclude_features: HashSet::new(), include_features: HashSet::new(), @@ -169,6 +184,50 @@ pub struct WorkspaceConfig { /// List of package names to exclude from the workspace analysis. #[serde(default)] pub exclude_packages: HashSet, + /// Target triples checked by default for the whole workspace. + /// + /// An empty list means "no configured target list"; behavior falls back to + /// the existing single effective target detection path. Package-level + /// `targets` override (do not merge with) this list. + #[serde(default)] + pub targets: Vec, + /// Target-specific workspace overrides keyed by Cargo-style cfg expressions. + /// + /// These may patch `exclude_packages` only; they select which workspace + /// packages participate for one already-selected target. + #[serde(default)] + pub target: BTreeMap, + /// Per-subcommand target-capability opt-in for aliases and custom commands. + /// + /// Built-in Cargo subcommands get their target capability from code and + /// ignore this table. Unknown aliases (e.g. `lint`) are denied configured + /// targets unless listed here with `targets = true`. + #[serde(default)] + pub subcommands: BTreeMap, +} + +/// Target-specific workspace override. +/// +/// Keyed by Cargo-style cfg expressions, e.g. `cfg(target_arch = "wasm32")`. +/// Workspace target overrides may patch `exclude_packages` only. +#[derive(Serialize, Deserialize, Default, Debug, Clone)] +pub struct WorkspaceTargetOverride { + /// Patch operations for [`WorkspaceConfig::exclude_packages`]. + #[serde(default)] + pub exclude_packages: Option, +} + +/// Workspace-level target-capability opt-in for a single command token. +/// +/// A plain `bool` (default `false`) is enough in v1: only unknown aliases +/// consult this table and the default is deny, so "omitted" and +/// `targets = false` are indistinguishable in practice. +#[derive(Serialize, Deserialize, Default, Debug, Clone)] +pub struct CommandTargetCapability { + /// When `true`, cargo-fc may expand configured target lists and inject + /// `--target ` for this command. + #[serde(default)] + pub targets: bool, } /// Deprecated configuration keys kept for backwards compatibility. diff --git a/src/config/resolve.rs b/src/config/resolve.rs index d13f9b7..5588f2f 100644 --- a/src/config/resolve.rs +++ b/src/config/resolve.rs @@ -34,6 +34,9 @@ pub fn resolve_config( if matched.is_empty() { let mut out = base.clone(); out.target.clear(); + // `targets` is a selection field consumed before resolution; clear it so + // the resolved (feature-matrix) config never carries it. + out.targets = None; return Ok(out); } @@ -72,6 +75,9 @@ pub fn resolve_config( // Remove target metadata from the resolved config. out.target.clear(); out.deprecated = DeprecatedConfig::default(); + // `targets` is a selection field consumed before resolution; clear it so the + // resolved (feature-matrix) config never carries it. + out.targets = None; Ok(out) } diff --git a/src/lib.rs b/src/lib.rs index b72d337..9cdd7a4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,8 @@ pub mod package; pub mod runner; /// Target triple handling and host/flag based detection. pub mod target; +/// Target selection and target-plan construction. +pub mod target_plan; /// IO utilities. pub mod tee; /// Workspace-level configuration and package discovery. @@ -38,7 +40,7 @@ use cli::cargo_subcommand; use color_eyre::eyre; use runner::print_feature_combination_error; use std::process; -use target::{RustcTargetDetector, TargetDetector}; +use target::RustcTargetEnvironment; /// Yellow+bold color spec used by the [`print_warning!`] macro. static WARNING_COLOR: std::sync::LazyLock = std::sync::LazyLock::new(|| { @@ -48,6 +50,14 @@ static WARNING_COLOR: std::sync::LazyLock = std::sync::Laz spec }); +/// Cyan+bold color spec used by the [`print_note!`] macro. +static NOTE_COLOR: std::sync::LazyLock = std::sync::LazyLock::new(|| { + let mut spec = termcolor::ColorSpec::new(); + spec.set_fg(Some(termcolor::Color::Cyan)); + spec.set_bold(true); + spec +}); + /// Print a colored warning to stderr. /// /// Formats as `warning: ` with the `warning:` prefix in yellow. @@ -65,6 +75,22 @@ macro_rules! print_warning { } pub(crate) use print_warning; +/// Print a colored informational note to stderr. +/// +/// Formats as `note: ` with the `note:` prefix in cyan. Used for +/// non-fatal mode fallbacks/no-ops such as `--aggregate-targets` adjustments. +macro_rules! print_note { + ($($arg:tt)*) => {{ + use std::io::Write as _; + use termcolor::WriteColor as _; + let mut stderr = termcolor::StandardStream::stderr(termcolor::ColorChoice::Auto); + let _ = stderr.set_color(&$crate::NOTE_COLOR); + let _ = write!(&mut stderr, "note"); + let _ = stderr.reset(); + let _ = writeln!(&mut stderr, ": {}", format_args!($($arg)*)); + }}; +} + /// Whether to warn when the cargo subcommand is not one of the known commands /// (`build`, `test`, `run`, `check`, `doc`, `clippy`). Disabled by default /// because cargo aliases (e.g. `cargo lint`) are common and the tool handles @@ -151,52 +177,69 @@ pub fn run(bin_name: &str) -> eyre::Result<()> { cmd.manifest_path(manifest_path); } let metadata = cmd.exec()?; - let mut packages = metadata.packages_for_fc()?; - // When `--manifest-path` points to a workspace member, `cargo metadata` - // still returns the entire workspace. Unless the user explicitly selected - // packages via `-p/--package`, default to only processing the root package - // resolved by Cargo for the given manifest. - if options.manifest_path.is_some() - && options.packages.is_empty() - && let Some(root) = metadata.root_package() - { - packages.retain(|p| p.id == root.id); - } - - // Filter excluded packages via CLI arguments - packages.retain(|p| !options.exclude_packages.contains(p.name.as_str())); - - if options.only_packages_with_lib_target { - // Filter only packages with a library target - packages.retain(|p| { - p.targets - .iter() - .any(|t| t.kind.contains(&cargo_metadata::TargetKind::Lib)) - }); - } - - // Filter packages based on CLI options - if !options.packages.is_empty() { - packages.retain(|p| options.packages.contains(p.name.as_str())); - } + let ws_config = metadata.workspace_config()?; + // Discover candidate packages without applying workspace exclusions; those + // (and their target-specific patches) are applied per target during + // planning. + let packages = select_candidate_packages(&metadata, &options)?; + + // Cache each selected package's base config once so planning and execution + // never re-read the manifest (which would duplicate deprecation warnings). + let configs: Vec = packages + .iter() + .map(|package| package.config()) + .collect::>>()?; + let selected: Vec> = packages + .iter() + .zip(&configs) + .map(|(package, config)| target_plan::SelectedPackage { package, config }) + .collect(); // Preserve the original String args for `--target` detection. let cargo_args_owned = cargo_args; let cargo_args: Vec<&str> = cargo_args_owned.iter().map(String::as_str).collect(); - let detector = RustcTargetDetector::default(); - let target = detector.detect_target(&cargo_args_owned)?; + // Parse an explicit `--target` only before `--`. + let cli_target = target::parse_cli_target(&cargo_args_owned); + + let capability_allowed = + resolve_capability_and_warn(&options, &cargo_args, &ws_config, &selected); + + let env = RustcTargetEnvironment::default(); let mut evaluator = RustcCfgEvaluator::default(); + let base_exclude = metadata.base_workspace_exclude_packages()?; + + let target_plans = target_plan::build_target_plans( + &selected, + &ws_config, + &base_exclude, + cli_target.as_deref(), + capability_allowed, + &env, + &mut evaluator, + )?; + let result = match options.command { Some(Command::Help | Command::Version) => Ok(None), Some(Command::FeatureMatrix { pretty }) => { + let plan_set = runner::build_execution_plans( + &target_plans, + &options, + options.packages_only, + &mut evaluator, + )?; + if options.aggregate_targets { + print_note!( + "--aggregate-targets has no effect for matrix output; matrix rows are always per target" + ); + } let matrix_opts = runner::MatrixOptions { pretty, packages_only: options.packages_only, no_prune_implied: options.no_prune_implied, }; - print_feature_matrix_for_target(&packages, &target, &mut evaluator, &matrix_opts) + runner::print_matrix_for_execution_plans(&plan_set, &matrix_opts) } None => { if WARN_UNKNOWN_SUBCOMMAND @@ -206,7 +249,10 @@ pub fn run(bin_name: &str) -> eyre::Result<()> { "`cargo {bin_name}` only supports cargo's `build`, `test`, `run`, `check`, `doc`, and `clippy` subcommands" ); } - run_cargo_command_for_target(&packages, cargo_args, &options, &target, &mut evaluator) + let plan_set = + runner::build_execution_plans(&target_plans, &options, false, &mut evaluator)?; + let mode = resolve_execution_mode(&options, &cargo_args, &plan_set); + runner::run_execution_plans(&plan_set, cargo_args, &options, mode) } }; @@ -223,12 +269,205 @@ pub fn run(bin_name: &str) -> eyre::Result<()> { } } +/// Discover candidate workspace packages and apply CLI-level package filters. +/// +/// Workspace `exclude_packages` (and its target-specific patches) are applied +/// later, per target, by the planner — not here. +fn select_candidate_packages<'a>( + metadata: &'a cargo_metadata::Metadata, + options: &Options, +) -> eyre::Result> { + let mut packages = metadata.candidate_packages_for_fc()?; + + // When `--manifest-path` points to a workspace member, `cargo metadata` + // still returns the entire workspace. Unless the user explicitly selected + // packages via `-p/--package`, default to only processing the root package + // resolved by Cargo for the given manifest. + if options.manifest_path.is_some() + && options.packages.is_empty() + && let Some(root) = metadata.root_package() + { + packages.retain(|p| p.id == root.id); + } + + // Filter excluded packages via CLI arguments + packages.retain(|p| !options.exclude_packages.contains(p.name.as_str())); + + if options.only_packages_with_lib_target { + // Filter only packages with a library target + packages.retain(|p| { + p.targets + .iter() + .any(|t| t.kind.contains(&cargo_metadata::TargetKind::Lib)) + }); + } + + // Filter packages based on CLI options + if !options.packages.is_empty() { + packages.retain(|p| options.packages.contains(p.name.as_str())); + } + + Ok(packages) +} + +/// Resolve the selected command's target capability and warn (once) if +/// configured targets exist but the command can not accept them. +/// +/// `matrix` is not a forwarded cargo command: it always uses configured target +/// planning. The warning is driven from raw config state, not from the planned +/// targets after capability filtering. +fn resolve_capability_and_warn( + options: &Options, + cargo_args: &[&str], + ws_config: &config::WorkspaceConfig, + selected: &[target_plan::SelectedPackage<'_>], +) -> bool { + let is_matrix = matches!(options.command, Some(Command::FeatureMatrix { .. })); + let (capability_allowed, policy) = if is_matrix { + (true, None) + } else { + let token = cli::cargo_subcommand_token(cargo_args); + let policy = cli::resolve_command_target_policy(token.as_deref(), &ws_config.subcommands); + (policy.targets.is_allowed(), Some(policy)) + }; + + let has_raw_configured_targets = !ws_config.targets.is_empty() + || selected + .iter() + .any(|s| s.config.targets.as_ref().is_some_and(|t| !t.is_empty())); + if !capability_allowed + && has_raw_configured_targets + && let Some(policy) = &policy + && !policy.command_name.is_empty() + { + print_warning!( + "not passing configured targets to cargo command `{}` because it has no targets capability", + policy.command_name, + ); + eprintln!( + "hint: add [workspace.metadata.cargo-fc.subcommands.{}] targets = true if this command accepts --target", + policy.command_name, + ); + } + + capability_allowed +} + +/// Resolve the effective target execution mode, emitting a note when an +/// explicitly requested `--aggregate-targets` falls back to serial or is a +/// no-op. +fn resolve_execution_mode( + options: &Options, + cargo_args: &[&str], + plan_set: &runner::ExecutionPlanSet<'_>, +) -> runner::TargetExecutionMode { + use runner::TargetExecutionMode; + + if !options.aggregate_targets { + return TargetExecutionMode::SerialPerTarget; + } + + if plan_set.plans.len() <= 1 { + print_note!("--aggregate-targets has no effect for a single target; running normally"); + return TargetExecutionMode::SerialPerTarget; + } + + if cargo_subcommand(cargo_args) == cli::CargoSubcommand::Run { + print_note!( + "--aggregate-targets does not apply to `run` (cargo runs one target at a time); running targets serially" + ); + return TargetExecutionMode::SerialPerTarget; + } + + if plan_set.show_pruned { + print_note!( + "--aggregate-targets is disabled because pruned summaries are target-specific; running targets serially" + ); + return TargetExecutionMode::SerialPerTarget; + } + + TargetExecutionMode::Aggregate +} + #[cfg(test)] mod test { use super::*; use color_eyre::eyre; use serde_json::json; + fn execution_plan_set( + targets: &[&str], + show_pruned: bool, + ) -> runner::ExecutionPlanSet<'static> { + runner::ExecutionPlanSet { + plans: targets + .iter() + .map(|target| runner::ExecutionPlan { + target: target::TargetTriple((*target).to_string()), + package_plans: Vec::new(), + }) + .collect(), + show_pruned, + show_target: targets.len() > 1, + } + } + + #[test] + fn aggregate_execution_mode_selected_for_supported_multi_target_command() { + let options = Options { + aggregate_targets: true, + ..Options::default() + }; + let plan_set = execution_plan_set(&["t1", "t2"], false); + + assert_eq!( + resolve_execution_mode(&options, &["check"], &plan_set), + runner::TargetExecutionMode::Aggregate + ); + } + + #[test] + fn aggregate_execution_mode_falls_back_for_run() { + let options = Options { + aggregate_targets: true, + ..Options::default() + }; + let plan_set = execution_plan_set(&["t1", "t2"], false); + + assert_eq!( + resolve_execution_mode(&options, &["run"], &plan_set), + runner::TargetExecutionMode::SerialPerTarget + ); + } + + #[test] + fn aggregate_execution_mode_falls_back_for_pruned_summaries() { + let options = Options { + aggregate_targets: true, + ..Options::default() + }; + let plan_set = execution_plan_set(&["t1", "t2"], true); + + assert_eq!( + resolve_execution_mode(&options, &["check"], &plan_set), + runner::TargetExecutionMode::SerialPerTarget + ); + } + + #[test] + fn aggregate_execution_mode_is_noop_for_single_target() { + let options = Options { + aggregate_targets: true, + ..Options::default() + }; + let plan_set = execution_plan_set(&["t1"], false); + + assert_eq!( + resolve_execution_mode(&options, &["check"], &plan_set), + runner::TargetExecutionMode::SerialPerTarget + ); + } + #[test] fn find_metadata_value_returns_none_for_empty_object() { let meta = json!({}); diff --git a/src/runner.rs b/src/runner.rs index 4f69a54..65b0e11 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,15 +1,19 @@ //! Cargo command execution, output parsing, summary printing, and matrix output. use crate::DEFAULT_PKG_METADATA_SECTION; +use crate::cfg_eval::CfgEvaluator; use crate::cli::{CargoSubcommand, Options, cargo_subcommand}; +use crate::config::Config; +use crate::implication::PrunedCombination; use crate::package::{FeatureCombinationError, Package}; use crate::print_warning; -use crate::target::TargetTriple; +use crate::target::{EffectiveTarget, TargetSource, TargetTriple}; +use crate::target_plan::{PlannedPackage, TargetPlan, TargetPlans}; use color_eyre::eyre; use itertools::Itertools; use regex::Regex; -use std::collections::HashSet; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::io::{self, Write}; use std::process; use std::sync::LazyLock; @@ -61,10 +65,39 @@ fn force_color(cmd: &mut process::Command) { cmd.env("FORCE_COLOR", "1"); } +/// Target display context for a summary entry and command header. +/// +/// `Hidden` preserves implicit single-host output, `Single` prints +/// `target = ...` (exact per-target attribution), and `Group` prints +/// `targets = [...]` for an aggregate multi-target invocation. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum SummaryTarget { + /// Implicit single-host run: no target field is shown. + Hidden, + /// A single concrete target with exact attribution. + Single(TargetTriple), + /// An aggregate group of targets sharing one Cargo invocation. + Group(Vec), +} + +impl SummaryTarget { + /// The `target = ...,` / `targets = [...],` field prefix shown inside the + /// `( ... )` of headers and summary entries, including the trailing + /// `", "`. Empty for [`SummaryTarget::Hidden`]. + fn field_prefix(&self) -> String { + match self { + Self::Hidden => String::new(), + Self::Single(triple) => format!("target = {triple}, "), + Self::Group(triples) => format!("targets = [{}], ", triples.iter().join(", ")), + } + } +} + /// Summary of the outcome for running (or pruning) a single feature set. #[derive(Debug, Clone)] pub struct Summary { package_name: String, + target: SummaryTarget, features: Vec, exit_code: Option, pedantic_success: bool, @@ -232,7 +265,7 @@ pub(crate) fn print_feature_combination_error(err: &FeatureCombinationError) { pub fn print_summary( summary: &[Summary], show_pruned: bool, - mut stdout: termcolor::StandardStream, + stdout: &mut termcolor::StandardStream, elapsed: Duration, ) -> ExitCode { let num_packages = summary @@ -240,21 +273,48 @@ pub fn print_summary( .map(|s| &s.package_name) .collect::>() .len(); + // Key executed/pruned combinations by (package, target, features) so that + // identical feature sets across targets do not collapse. let num_total = summary .iter() - .map(|s| (&s.package_name, s.features.iter().collect::>())) + .map(|s| { + ( + &s.package_name, + &s.target, + s.features.iter().collect::>(), + ) + }) .collect::>() .len(); let num_pruned = summary.iter().filter(|s| s.is_pruned()).count(); let num_executed = num_total - num_pruned; + let mut target_set: HashSet<&TargetTriple> = HashSet::new(); + for s in summary { + match &s.target { + SummaryTarget::Hidden => {} + SummaryTarget::Single(triple) => { + target_set.insert(triple); + } + SummaryTarget::Group(triples) => { + target_set.extend(triples.iter()); + } + } + } + let num_targets = target_set.len(); + let targets_clause = if num_targets > 1 { + format!(" across {num_targets} targets") + } else { + String::new() + }; + println!(); stdout.set_color(&CYAN).ok(); print!(" Finished "); stdout.reset().ok(); if num_pruned > 0 { print!( - "{num_executed} of {num_total} feature combination{} for {num_packages} package{} in {:.2}s", + "{num_executed} of {num_total} feature combination{} for {num_packages} package{}{targets_clause} in {:.2}s", if num_total > 1 { "s" } else { "" }, if num_packages > 1 { "s" } else { "" }, elapsed.as_secs_f64(), @@ -264,7 +324,7 @@ pub fn print_summary( stdout.reset().ok(); } else { print!( - "{num_total} feature combination{} for {num_packages} package{} in {:.2}s", + "{num_total} feature combination{} for {num_packages} package{}{targets_clause} in {:.2}s", if num_total > 1 { "s" } else { "" }, if num_packages > 1 { "s" } else { "" }, elapsed.as_secs_f64(), @@ -293,9 +353,13 @@ pub fn print_summary( warnings_width, suppressed_width, }; - print_summary_entry(s, &mut stdout, &fmt); + print_summary_entry(s, stdout, &fmt); if !s.pedantic_success { - first_bad_exit_code = first_bad_exit_code.or(s.exit_code); + let exit_code = match s.exit_code { + Some(code) if code != 0 => code, + _ => 1, + }; + first_bad_exit_code = first_bad_exit_code.or(Some(exit_code)); } } println!(); @@ -329,6 +393,7 @@ fn print_summary_entry(s: &Summary, stdout: &mut termcolor::StandardStream, fmt: stdout.reset().ok(); let feat = s.features.iter().join(", "); + let target = s.target.field_prefix(); let ew = fmt.errors_width; let ww = fmt.warnings_width; let sw = fmt.suppressed_width; @@ -337,12 +402,12 @@ fn print_summary_entry(s: &Summary, stdout: &mut termcolor::StandardStream, fmt: let ns = s.num_suppressed; if fmt.show_suppressed { print!( - "{} ( {ne:>ew$} errors, {nw:>ww$} warnings, {ns:>sw$} suppressed, features = [{feat}] )", + "{} ( {target}{ne:>ew$} errors, {nw:>ww$} warnings, {ns:>sw$} suppressed, features = [{feat}] )", s.package_name, ); } else { print!( - "{} ( {ne:>ew$} errors, {nw:>ww$} warnings, features = [{feat}] )", + "{} ( {target}{ne:>ew$} errors, {nw:>ww$} warnings, features = [{feat}] )", s.package_name, ); } @@ -365,9 +430,26 @@ struct Progress { width: usize, } +/// The per-combination inputs for one Cargo invocation. +struct Invocation<'a> { + package: &'a cargo_metadata::Package, + features: &'a [String], + /// Target triples cargo-fc must inject as `--target` (configured sources). + inject_targets: &'a [String], + /// Display/attribution context for the header and summary entry. + summary_target: &'a SummaryTarget, +} + +/// One aggregate-mode Cargo invocation after transposing target plans by package +/// and feature combination. +struct AggregateInvocationPlan<'a> { + package: &'a cargo_metadata::Package, + combo: Vec, + targets: Vec, +} + fn print_package_cmd( - package: &cargo_metadata::Package, - features: &[String], + inv: &Invocation<'_>, cargo_args: &[&str], all_args: &[&str], options: &Options, @@ -420,9 +502,10 @@ fn print_package_cmd( stdout.reset().ok(); } print!( - " {} ( features = [{}] )", - package.name, - features.iter().join(", ") + " {} ( {}features = [{}] )", + inv.package.name, + inv.summary_target.field_prefix(), + inv.features.iter().join(", ") ); if options.verbose { print!(" [cargo {}]", all_args.join(" ")); @@ -490,12 +573,14 @@ pub fn print_feature_matrix_for_target( let matrix: Vec = per_package_features .into_iter() .flat_map(|(name, config, features)| { + let target = target.as_str().to_string(); features.into_iter().map(move |ft| { use serde_json_merge::{iter::dfs::Dfs, merge::Merge}; let mut out = serde_json::json!(config.matrix); out.merge::(&serde_json::json!({ "name": name, + "target": target, "features": ft, })); out @@ -512,6 +597,196 @@ pub fn print_feature_matrix_for_target( Ok(None) } +/// A resolved, owned execution plan for one concrete target. +/// +/// After [`build_execution_plans`], execution owns a deterministic sequence of +/// resolved `(package, target, combinations, pruned)` units and needs neither a +/// [`CfgEvaluator`] nor package configs. +pub struct ExecutionPlan<'a> { + /// The concrete target triple this plan is for. + pub target: TargetTriple, + /// The per-package execution plans for this target, in plan order. + pub package_plans: Vec>, +} + +/// A resolved, owned execution plan for one package on one target. +pub struct PackageExecutionPlan<'a> { + /// The package. + pub package: &'a cargo_metadata::Package, + /// The concrete target and where it came from. + pub target: EffectiveTarget, + /// Feature combinations to execute, in deterministic (sorted) order. + pub combinations: Vec>, + /// Combinations pruned as implied by another combination. + pub pruned: Vec, + /// Resolved user matrix metadata for this package-target (matrix output + /// only; the executor ignores it). + pub matrix: HashMap, +} + +/// The full set of execution plans plus display flags. +pub struct ExecutionPlanSet<'a> { + /// Execution plans in deterministic target-plan order. + pub plans: Vec>, + /// Whether pruned combinations should be shown in the summary. + pub show_pruned: bool, + /// Whether the `target = ...` field should be shown (not the implicit + /// single-host default). + pub show_target: bool, +} + +impl ExecutionPlanSet<'_> { + /// The distinct target triples planned, in plan order. + #[must_use] + pub fn distinct_targets(&self) -> Vec { + self.plans.iter().map(|p| p.target.clone()).collect() + } +} + +/// Build owned execution plans from target plans. +/// +/// Resolves each package assignment's target-specific config from the cached +/// `PlannedPackage::config` (never re-reading the manifest), generates and +/// prunes feature combinations, and stores owned feature strings so execution +/// borrows nothing from temporary configs. +/// +/// When `packages_only` is set, feature generation is skipped (matrix +/// `--packages-only` only needs one `"default"` row per package-target). +/// +/// # Errors +/// +/// Returns an error if a package's config can not be resolved or its feature +/// combinations can not be generated. +pub fn build_execution_plans<'a>( + target_plans: &TargetPlans<'a>, + options: &Options, + packages_only: bool, + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result> { + let mut plans = Vec::with_capacity(target_plans.plans.len()); + let mut config_show_pruned = false; + + for target_plan in &target_plans.plans { + let mut package_plans = Vec::with_capacity(target_plan.packages.len()); + for planned in &target_plan.packages { + let config = crate::config::resolve::resolve_config( + planned.config, + &target_plan.target, + evaluator, + )?; + config_show_pruned = config_show_pruned || config.show_pruned; + + let (combinations, pruned) = if packages_only { + (Vec::new(), Vec::new()) + } else { + let all_combos = planned.package.feature_combinations(&config)?; + let prune_result = crate::implication::maybe_prune( + all_combos, + &planned.package.features, + &config, + options.no_prune_implied, + ); + // Own the feature strings before the resolved config is dropped. + let combinations: Vec> = prune_result + .keep + .into_iter() + .map(|combo| combo.into_iter().cloned().collect()) + .collect(); + (combinations, prune_result.pruned) + }; + + package_plans.push(PackageExecutionPlan { + package: planned.package, + target: planned.target.clone(), + combinations, + pruned, + matrix: config.matrix, + }); + } + plans.push(ExecutionPlan { + target: target_plan.target.clone(), + package_plans, + }); + } + + let show_pruned = options.show_pruned || config_show_pruned; + let show_target = target_plans.contains_configured_assignments || plans.len() > 1; + + Ok(ExecutionPlanSet { + plans, + show_pruned, + show_target, + }) +} + +/// Build the JSON feature-matrix rows for the given execution plans. +/// +/// Every row carries `name`, `target`, and `features`. Built-in fields win over +/// user `matrix` metadata that uses the same keys; because `target` is now a +/// reserved built-in key, a compatibility warning is emitted (once per package) +/// if user matrix metadata already defines it. +#[must_use] +pub fn build_matrix_rows( + plan_set: &ExecutionPlanSet, + packages_only: bool, +) -> Vec { + use serde_json_merge::{iter::dfs::Dfs, merge::Merge}; + + let mut rows = Vec::new(); + let mut warned_packages: HashSet = HashSet::new(); + + for plan in &plan_set.plans { + for pp in &plan.package_plans { + let name = pp.package.name.to_string(); + if pp.matrix.contains_key("target") && warned_packages.insert(name.clone()) { + print_warning!( + "package `{name}` matrix metadata defines the reserved `target` key; cargo-fc overwrites it with the configured target triple" + ); + } + + let features_list: Vec = if packages_only { + vec!["default".to_string()] + } else { + pp.combinations + .iter() + .map(|combo| combo.iter().join(",")) + .collect() + }; + + for ft in features_list { + let mut out = serde_json::json!(pp.matrix); + out.merge::(&serde_json::json!({ + "name": name, + "target": pp.target.triple.as_str(), + "features": ft, + })); + rows.push(out); + } + } + } + + rows +} + +/// Print a JSON feature matrix built from execution plans to stdout. +/// +/// # Errors +/// +/// Returns an error if serialization of the JSON matrix fails. +pub fn print_matrix_for_execution_plans( + plan_set: &ExecutionPlanSet, + options: &MatrixOptions, +) -> eyre::Result { + let rows = build_matrix_rows(plan_set, options.packages_only); + let matrix = if options.pretty { + serde_json::to_string_pretty(&rows) + } else { + serde_json::to_string(&rows) + }?; + println!("{matrix}"); + Ok(None) +} + /// Pre-computed state shared across all feature combinations in a single /// [`run_cargo_command_for_target`] invocation. struct RunContext<'a> { @@ -620,13 +895,14 @@ fn normalize_feature_selection_args(cargo_args: Vec<&str>) -> FeatureSelectionNo /// Run a single cargo invocation for one feature combination and collect /// its output into a [`Summary`]. fn run_single_combination( - package: &cargo_metadata::Package, - features: &[String], + inv: &Invocation<'_>, ctx: &RunContext<'_>, progress: Progress, seen_diagnostics: &mut HashSet, stdout: &mut StandardStream, ) -> eyre::Result { + let package = inv.package; + let features = inv.features; // We set the command working dir to the package manifest parent dir. // This works well for now, but one could also consider `--manifest-path` or `-p` let Some(working_dir) = package.manifest_path.parent() else { @@ -654,21 +930,17 @@ fn run_single_combination( if ctx.use_diagnostics_only { args.push(crate::diagnostics_only::MESSAGE_FORMAT); } + for triple in inv.inject_targets { + args.push("--target"); + args.push(triple.as_str()); + } let features_flag = format!("--features={}", features.iter().join(",")); if !ctx.missing_arguments { args.push("--no-default-features"); args.push(&features_flag); } args.extend_from_slice(ctx.extra_args); - print_package_cmd( - package, - features, - ctx.cargo_args, - &args, - ctx.options, - progress, - stdout, - ); + print_package_cmd(inv, ctx.cargo_args, &args, ctx.options, progress, stdout); cmd.args(&args).current_dir(working_dir); if ctx.use_diagnostics_only { @@ -723,6 +995,7 @@ fn run_single_combination( let summary = Summary { features: features.to_vec(), + target: inv.summary_target.clone(), num_errors: result.num_errors, num_warnings: result.num_warnings, num_suppressed: result.num_suppressed, @@ -738,73 +1011,86 @@ fn run_single_combination( }) } -/// Pre-computed plan for running one package's feature combinations. -struct PackagePlan<'a> { - package: &'a cargo_metadata::Package, - keep: Vec>, - pruned: Vec, -} - -/// Resolve and prune every package's feature combinations up front. +/// Run a cargo command for all requested packages and feature combinations on +/// one concrete target. /// -/// Returns the per-package run plans together with whether any package's -/// resolved config requested that pruned combinations be shown in the summary. +/// This is the backward-compatible single-target entry point for library +/// consumers that control target resolution themselves. Internally it wraps a +/// one-item [`TargetPlans`] (source [`TargetSource::Cli`], so no `--target` is +/// injected and the implicit single-host output is preserved) and runs it +/// through the serial executor. /// /// # Errors /// -/// Returns an error if a package's config can not be resolved or its feature -/// combinations can not be generated (e.g. too many features). -fn plan_feature_combinations<'a>( - packages: &[&'a cargo_metadata::Package], +/// Returns an error if a cargo process can not be spawned or if IO operations +/// fail while reading cargo's output. +pub fn run_cargo_command_for_target( + packages: &[&cargo_metadata::Package], + cargo_args: Vec<&str>, options: &Options, target: &TargetTriple, - evaluator: &mut impl crate::cfg_eval::CfgEvaluator, -) -> eyre::Result<(Vec>, bool)> { - let mut plans = Vec::with_capacity(packages.len()); - let mut show_pruned = false; - for &package in packages { - let base_config = package.config()?; - let config = crate::config::resolve::resolve_config(&base_config, target, evaluator)?; - show_pruned = show_pruned || config.show_pruned; - - let all_combos = package.feature_combinations(&config)?; - let prune_result = crate::implication::maybe_prune( - all_combos, - &package.features, - &config, - options.no_prune_implied, - ); - // The generated combinations may borrow from the package config - // (`include_features`), so own them before the config is dropped. - let keep: Vec> = prune_result - .keep - .into_iter() - .map(|combo| combo.into_iter().cloned().collect()) - .collect(); - plans.push(PackagePlan { - package, - keep, - pruned: prune_result.pruned, - }); - } - Ok((plans, show_pruned)) + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result { + let configs: Vec = packages + .iter() + .map(|package| package.config()) + .collect::>>()?; + + let target_plans = TargetPlans { + plans: vec![TargetPlan { + target: target.clone(), + packages: packages + .iter() + .zip(&configs) + .map(|(package, config)| PlannedPackage { + package, + config, + target: EffectiveTarget { + triple: target.clone(), + source: TargetSource::Cli, + }, + }) + .collect(), + }], + contains_configured_assignments: false, + }; + + let plan_set = build_execution_plans(&target_plans, options, false, evaluator)?; + run_execution_plans( + &plan_set, + cargo_args, + options, + TargetExecutionMode::SerialPerTarget, + ) } -/// Run a cargo command for all requested packages and feature combinations. +/// Execution mode over the same execution plans. /// -/// This is useful for library consumers that want to control target -/// resolution themselves, e.g. when cross-compiling. +/// Both modes are single-threaded and stream live output; they differ only in +/// how targets map onto Cargo invocations and how summary entries are keyed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TargetExecutionMode { + /// Default: one invocation per `(package, target, combo)`, exact per-target + /// attribution. + SerialPerTarget, + /// `--aggregate-targets`: one invocation per `(package, combo)` carrying all + /// that combo's targets as repeated `--target` flags, group-level + /// attribution. + Aggregate, +} + +/// Set up shared Cargo invocation context and run the execution plans in the +/// given mode. /// /// # Errors /// /// Returns an error if a cargo process can not be spawned or if IO operations /// fail while reading cargo's output. -pub fn run_cargo_command_for_target( - packages: &[&cargo_metadata::Package], +pub fn run_execution_plans( + plan_set: &ExecutionPlanSet, mut cargo_args: Vec<&str>, options: &Options, - target: &TargetTriple, - evaluator: &mut impl crate::cfg_eval::CfgEvaluator, + mode: TargetExecutionMode, ) -> eyre::Result { let start = Instant::now(); @@ -857,77 +1143,227 @@ pub fn run_cargo_command_for_target( }; let mut stdout = StandardStream::stdout(ColorChoice::Auto); - let mut summary: Vec = Vec::new(); let mut seen_diagnostics: HashSet = HashSet::new(); - let (plans, config_show_pruned) = - plan_feature_combinations(packages, options, target, evaluator)?; - - // show_pruned is a display concern for the global summary, so if any - // package enables it via config we show pruned entries for all packages. - let show_pruned = options.show_pruned || config_show_pruned; + match mode { + TargetExecutionMode::SerialPerTarget => { + execute_serial(plan_set, &ctx, &mut seen_diagnostics, &mut stdout, start) + } + TargetExecutionMode::Aggregate => { + execute_aggregate(plan_set, &ctx, &mut seen_diagnostics, &mut stdout, start) + } + } +} - let total: usize = plans.iter().map(|plan| plan.keep.len()).sum(); +/// Serial per-target execution: one Cargo invocation per +/// `(package, target, combo)`. +fn execute_serial( + plan_set: &ExecutionPlanSet, + ctx: &RunContext<'_>, + seen_diagnostics: &mut HashSet, + stdout: &mut StandardStream, + start: Instant, +) -> eyre::Result { + let mut summary: Vec = Vec::new(); + let total: usize = plan_set + .plans + .iter() + .flat_map(|plan| plan.package_plans.iter()) + .map(|pp| pp.combinations.len()) + .sum(); let width = total.to_string().len(); let mut index = 0; - for plan in plans { - let pkg_start = summary.len(); - for combo in &plan.keep { - index += 1; - let result = run_single_combination( - plan.package, - combo, - &ctx, - Progress { - index, - total, - width, - }, - &mut seen_diagnostics, - &mut stdout, - )?; - let should_stop = options.fail_fast && !result.summary.pedantic_success; - let exit_code = result.summary.exit_code; - summary.push(result.summary); - if should_stop { - if options.summary_only { - io::copy(&mut io::Cursor::new(result.colored_output), &mut stdout)?; - stdout.flush().ok(); + for plan in &plan_set.plans { + for pp in &plan.package_plans { + let summary_target = if plan_set.show_target { + SummaryTarget::Single(plan.target.clone()) + } else { + SummaryTarget::Hidden + }; + let inject: Vec = if pp.target.source.should_inject_target_arg() { + vec![pp.target.triple.0.clone()] + } else { + Vec::new() + }; + + let pkg_start = summary.len(); + for combo in &pp.combinations { + index += 1; + let result = run_single_combination( + &Invocation { + package: pp.package, + features: combo, + inject_targets: &inject, + summary_target: &summary_target, + }, + ctx, + Progress { + index, + total, + width, + }, + seen_diagnostics, + stdout, + )?; + let should_stop = ctx.options.fail_fast && !result.summary.pedantic_success; + let exit_code = result.summary.exit_code; + summary.push(result.summary); + if should_stop { + if ctx.options.summary_only { + io::copy(&mut io::Cursor::new(result.colored_output), stdout)?; + stdout.flush().ok(); + } + let code = + print_summary(&summary, plan_set.show_pruned, stdout, start.elapsed()) + .or(exit_code) + .unwrap_or(1); + return Ok(Some(code)); } - let code = print_summary(&summary, show_pruned, stdout, start.elapsed()) - .or(exit_code) - .unwrap_or(1); - return Ok(Some(code)); } + + append_pruned_summaries( + &mut summary, + pkg_start, + pp.package.name.as_ref(), + &summary_target, + pp.pruned.clone(), + ); } + } - append_pruned_summaries( - &mut summary, - pkg_start, - plan.package.name.as_ref(), - plan.pruned, - ); + Ok(print_summary( + &summary, + plan_set.show_pruned, + stdout, + start.elapsed(), + )) +} + +/// Aggregate execution: one Cargo invocation per `(package, combo)` carrying +/// every target that shares the combo as repeated `--target` flags. +fn execute_aggregate( + plan_set: &ExecutionPlanSet, + ctx: &RunContext<'_>, + seen_diagnostics: &mut HashSet, + stdout: &mut StandardStream, + start: Instant, +) -> eyre::Result { + let invocations = aggregate_invocation_plans(plan_set); + let total = invocations.len(); + let width = total.to_string().len(); + let mut summary: Vec = Vec::new(); + + for (zero_index, inv_plan) in invocations.iter().enumerate() { + let index = zero_index + 1; + let triples: Vec = + inv_plan.targets.iter().map(|t| t.triple.clone()).collect(); + let summary_target = match triples.as_slice() { + [single] => SummaryTarget::Single(single.clone()), + _ => SummaryTarget::Group(triples.clone()), + }; + let inject: Vec = inv_plan + .targets + .iter() + .filter(|t| t.source.should_inject_target_arg()) + .map(|t| t.triple.0.clone()) + .collect(); + + let result = run_single_combination( + &Invocation { + package: inv_plan.package, + features: &inv_plan.combo, + inject_targets: &inject, + summary_target: &summary_target, + }, + ctx, + Progress { + index, + total, + width, + }, + seen_diagnostics, + stdout, + )?; + let should_stop = ctx.options.fail_fast && !result.summary.pedantic_success; + let exit_code = result.summary.exit_code; + summary.push(result.summary); + if should_stop { + if ctx.options.summary_only { + io::copy(&mut io::Cursor::new(result.colored_output), stdout)?; + stdout.flush().ok(); + } + let code = print_summary(&summary, plan_set.show_pruned, stdout, start.elapsed()) + .or(exit_code) + .unwrap_or(1); + return Ok(Some(code)); + } } Ok(print_summary( &summary, - show_pruned, + plan_set.show_pruned, stdout, start.elapsed(), )) } -/// Append pruned summaries for a single package, looking up the equivalent -/// combo's error/warning counts from already-executed summaries, then sort -/// all entries for this package by features for interleaved display. +/// Transpose per-target execution plans into aggregate-mode invocations. +/// +/// The resulting order is package first-appearance order, sorted canonical combo +/// order, then target-plan order within each combo's target group. +fn aggregate_invocation_plans<'a>( + plan_set: &'a ExecutionPlanSet<'a>, +) -> Vec> { + let mut package_order: Vec<&cargo_metadata::Package> = Vec::new(); + let mut seen_packages: HashSet = HashSet::new(); + let mut grouped: HashMap, Vec>> = HashMap::new(); + + for plan in &plan_set.plans { + for pp in &plan.package_plans { + let id = pp.package.id.repr.clone(); + if seen_packages.insert(id.clone()) { + package_order.push(pp.package); + } + let entry = grouped.entry(id).or_default(); + for combo in &pp.combinations { + entry + .entry(combo.clone()) + .or_default() + .push(pp.target.clone()); + } + } + } + + let mut invocations = Vec::new(); + for package in package_order { + let Some(combos) = grouped.remove(&package.id.repr) else { + continue; + }; + for (combo, targets) in combos { + invocations.push(AggregateInvocationPlan { + package, + combo, + targets, + }); + } + } + + invocations +} + +/// Append pruned summaries for a single `(package, target)` block, looking up +/// the equivalent combo's error/warning counts from already-executed summaries +/// scoped to that block, then sort the block by features for interleaved +/// display. fn append_pruned_summaries( summary: &mut Vec, pkg_start: usize, package_name: &str, - pruned: Vec, + summary_target: &SummaryTarget, + pruned: Vec, ) { - let executed: std::collections::HashMap, Summary> = summary + let executed: HashMap, Summary> = summary .get(pkg_start..) .unwrap_or_default() .iter() @@ -941,6 +1377,7 @@ fn append_pruned_summaries( }; summary.push(Summary { package_name: package_name.to_string(), + target: summary_target.clone(), features: p.features, equivalent_to: Some(p.equivalent_to), num_errors: equiv.num_errors, @@ -959,11 +1396,15 @@ fn append_pruned_summaries( #[cfg(test)] mod test { use super::{ - error_counts, normalize_feature_selection_args, plan_feature_combinations, warning_counts, + ExecutionPlan, ExecutionPlanSet, PackageExecutionPlan, Summary, SummaryTarget, + aggregate_invocation_plans, build_execution_plans, error_counts, + normalize_feature_selection_args, print_summary, warning_counts, }; use crate::cfg_eval::CfgEvaluator; use crate::cli::{CargoSubcommand, Options}; - use crate::target::TargetTriple; + use crate::package::Package as _; + use crate::target::{EffectiveTarget, TargetSource, TargetTriple}; + use crate::target_plan::{PlannedPackage, TargetPlan, TargetPlans}; use color_eyre::eyre; use similar_asserts::assert_eq as sim_assert_eq; @@ -980,6 +1421,173 @@ mod test { values.iter().copied().map(String::from).collect() } + fn package(name: &str) -> eyre::Result { + use cargo_metadata::{PackageBuilder, PackageId, PackageName}; + use semver::Version; + use std::str::FromStr as _; + + Ok(PackageBuilder::new( + PackageName::from_str(name)?, + Version::parse("0.1.0")?, + PackageId { + repr: name.to_string(), + }, + "", + ) + .build()?) + } + + fn effective_target(triple: &str) -> EffectiveTarget { + EffectiveTarget { + triple: TargetTriple(triple.to_string()), + source: TargetSource::WorkspaceConfig, + } + } + + fn summary_with_failure(exit_code: Option, pedantic_success: bool) -> Summary { + Summary { + package_name: "pkg".to_string(), + target: SummaryTarget::Hidden, + features: Vec::new(), + exit_code, + pedantic_success, + num_warnings: usize::from(!pedantic_success), + num_errors: 0, + num_suppressed: 0, + equivalent_to: None, + } + } + + #[test] + fn summary_target_field_prefix() { + sim_assert_eq!(SummaryTarget::Hidden.field_prefix(), ""); + sim_assert_eq!( + SummaryTarget::Single(TargetTriple("t-a".to_string())).field_prefix(), + "target = t-a, " + ); + sim_assert_eq!( + SummaryTarget::Group(vec![ + TargetTriple("t-a".to_string()), + TargetTriple("t-b".to_string()), + ]) + .field_prefix(), + "targets = [t-a, t-b], " + ); + } + + #[test] + fn print_summary_returns_one_for_pedantic_warning_exit_zero() { + let summary = vec![summary_with_failure(Some(0), false)]; + let mut stdout = termcolor::StandardStream::stdout(termcolor::ColorChoice::Never); + + let exit = print_summary(&summary, false, &mut stdout, std::time::Duration::ZERO); + + sim_assert_eq!(exit, Some(1)); + } + + #[test] + fn print_summary_returns_one_for_failure_without_exit_code() { + let summary = vec![summary_with_failure(None, false)]; + let mut stdout = termcolor::StandardStream::stdout(termcolor::ColorChoice::Never); + + let exit = print_summary(&summary, false, &mut stdout, std::time::Duration::ZERO); + + sim_assert_eq!(exit, Some(1)); + } + + #[test] + fn print_summary_preserves_nonzero_failure_exit_code() { + let summary = vec![summary_with_failure(Some(101), false)]; + let mut stdout = termcolor::StandardStream::stdout(termcolor::ColorChoice::Never); + + let exit = print_summary(&summary, false, &mut stdout, std::time::Duration::ZERO); + + sim_assert_eq!(exit, Some(101)); + } + + #[test] + fn aggregate_invocation_plans_group_by_package_combo_and_target_order() -> eyre::Result<()> { + let package_a = package("a")?; + let package_b = package("b")?; + let plan_set = ExecutionPlanSet { + plans: vec![ + ExecutionPlan { + target: TargetTriple("t1".to_string()), + package_plans: vec![ + PackageExecutionPlan { + package: &package_a, + target: effective_target("t1"), + combinations: vec![string_vec(&["b"]), string_vec(&[])], + pruned: Vec::new(), + matrix: std::collections::HashMap::new(), + }, + PackageExecutionPlan { + package: &package_b, + target: effective_target("t1"), + combinations: vec![string_vec(&["z"])], + pruned: Vec::new(), + matrix: std::collections::HashMap::new(), + }, + ], + }, + ExecutionPlan { + target: TargetTriple("t2".to_string()), + package_plans: vec![ + PackageExecutionPlan { + package: &package_a, + target: effective_target("t2"), + combinations: vec![string_vec(&[]), string_vec(&["a"])], + pruned: Vec::new(), + matrix: std::collections::HashMap::new(), + }, + PackageExecutionPlan { + package: &package_b, + target: effective_target("t2"), + combinations: vec![string_vec(&["z"])], + pruned: Vec::new(), + matrix: std::collections::HashMap::new(), + }, + ], + }, + ], + show_pruned: false, + show_target: true, + }; + + let simplified: Vec<_> = aggregate_invocation_plans(&plan_set) + .into_iter() + .map(|inv| { + ( + inv.package.name.to_string(), + inv.combo, + inv.targets + .into_iter() + .map(|target| target.triple.0) + .collect::>(), + ) + }) + .collect(); + + sim_assert_eq!( + simplified, + vec![ + ( + "a".to_string(), + string_vec(&[]), + vec!["t1".to_string(), "t2".to_string()] + ), + ("a".to_string(), string_vec(&["a"]), vec!["t2".to_string()]), + ("a".to_string(), string_vec(&["b"]), vec!["t1".to_string()]), + ( + "b".to_string(), + string_vec(&["z"]), + vec!["t1".to_string(), "t2".to_string()] + ), + ], + ); + Ok(()) + } + #[test] fn error_regex_single_mod_multiple_errors() { let stderr = include_str!("../test-data/single_mod_multiple_errors_stderr.txt"); @@ -1011,7 +1619,7 @@ mod test { } #[test] - fn plan_feature_combinations_keeps_pruned_entries_for_summary() -> eyre::Result<()> { + fn build_execution_plans_keeps_pruned_entries_for_summary() -> eyre::Result<()> { let mut package = crate::package::test::package_with_metadata( &["A", "B", "C"], "cargo-fc", @@ -1022,21 +1630,38 @@ mod test { }; implied_features.push("A".to_string()); - let packages = vec![&package]; + let config = package.config()?; + let target_plans = TargetPlans { + plans: vec![TargetPlan { + target: TargetTriple("test-target".to_string()), + packages: vec![PlannedPackage { + package: &package, + config: &config, + target: EffectiveTarget { + triple: TargetTriple("test-target".to_string()), + source: TargetSource::Cli, + }, + }], + }], + contains_configured_assignments: false, + }; + let mut evaluator = StubEval; - let (plans, show_pruned) = plan_feature_combinations( - &packages, - &Options::default(), - &TargetTriple("test-target".to_string()), - &mut evaluator, - )?; + let plan_set = + build_execution_plans(&target_plans, &Options::default(), false, &mut evaluator)?; - assert!(show_pruned); - let [plan] = plans.as_slice() else { - eyre::bail!("expected one package plan, got {}", plans.len()); + assert!(plan_set.show_pruned); + let [plan] = plan_set.plans.as_slice() else { + eyre::bail!("expected one execution plan, got {}", plan_set.plans.len()); + }; + let [pp] = plan.package_plans.as_slice() else { + eyre::bail!( + "expected one package execution plan, got {}", + plan.package_plans.len() + ); }; sim_assert_eq!( - &plan.keep, + &pp.combinations, &vec![ string_vec(&[]), string_vec(&["A"]), @@ -1047,7 +1672,7 @@ mod test { ], ); - let pruned: Vec<_> = plan + let pruned: Vec<_> = pp .pruned .iter() .map(|p| (p.features.clone(), p.equivalent_to.clone())) diff --git a/src/target.rs b/src/target.rs index 3aabbfc..13138a5 100644 --- a/src/target.rs +++ b/src/target.rs @@ -2,7 +2,7 @@ use color_eyre::eyre::{self, WrapErr}; use std::process::Command; /// A Rust target triple. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct TargetTriple(pub String); impl TargetTriple { @@ -13,6 +13,161 @@ impl TargetTriple { } } +impl std::fmt::Display for TargetTriple { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +/// Where a package's effective target came from in the precedence chain. +/// +/// Carried per package-target assignment so that injection and output +/// decisions stay local to the assignment instead of relying on plan-wide +/// boolean drift (`is_configured`, `should_inject`, `is_multi_target`). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum TargetSource { + /// Explicit Cargo CLI `--target ` (already forwarded to Cargo). + Cli, + /// Package-level `targets` list (cargo-fc must inject `--target`). + PackageConfig, + /// Workspace-level `targets` list (cargo-fc must inject `--target`). + WorkspaceConfig, + /// `CARGO_BUILD_TARGET` (Cargo sees the env var; no injection needed). + CargoBuildTargetEnv, + /// Host target from `rustc -vV` (keep current no-`--target` behavior). + Host, +} + +impl TargetSource { + /// Whether cargo-fc must inject a `--target ` flag for this source. + #[must_use] + pub fn should_inject_target_arg(self) -> bool { + matches!(self, Self::PackageConfig | Self::WorkspaceConfig) + } + + /// Whether this assignment came from configured target metadata. + #[must_use] + pub fn is_configured(self) -> bool { + matches!(self, Self::PackageConfig | Self::WorkspaceConfig) + } +} + +/// A concrete target together with where it came from. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EffectiveTarget { + /// The concrete target triple. + pub triple: TargetTriple, + /// Where this target was selected from. + pub source: TargetSource, +} + +/// Adapter providing the ambient target environment for planning. +/// +/// Abstracted behind a trait so that target planning is unit-testable without +/// reading the real environment or invoking `rustc`. +pub trait TargetEnvironment { + /// The `CARGO_BUILD_TARGET` triple, if set and non-empty. + fn cargo_build_target(&self) -> Option; + /// The host target triple from `rustc -vV`. + /// + /// # Errors + /// + /// Returns an error if the host target cannot be determined. + fn host_target(&self) -> eyre::Result; +} + +/// Production [`TargetEnvironment`] backed by the process environment and +/// `rustc -vV`. +#[derive(Debug, Clone)] +pub struct RustcTargetEnvironment { + env: E, +} + +impl Default for RustcTargetEnvironment { + fn default() -> Self { + Self { env: ProcessEnv } + } +} + +impl RustcTargetEnvironment { + /// Create an environment adapter over a custom [`Env`]. + pub fn with_env(env: E) -> Self { + Self { env } + } +} + +impl TargetEnvironment for RustcTargetEnvironment { + fn cargo_build_target(&self) -> Option { + let triple = self.env.var("CARGO_BUILD_TARGET")?; + let triple = triple.trim(); + if triple.is_empty() { + None + } else { + Some(triple.to_string()) + } + } + + fn host_target(&self) -> eyre::Result { + host_triple() + } +} + +/// Detect the host target triple via `rustc -vV`. +/// +/// # Errors +/// +/// Returns an error if `rustc` cannot be invoked or its output cannot be +/// parsed. +pub fn host_triple() -> eyre::Result { + let output = Command::new("rustc") + .arg("-vV") + .output() + .wrap_err("failed to invoke rustc to detect host target")?; + + if !output.status.success() { + eyre::bail!("rustc -vV failed with exit code {:?}", output.status.code()); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + for line in stdout.lines() { + if let Some(host) = line.strip_prefix("host: ") { + let host = host.trim(); + if host.is_empty() { + continue; + } + return Ok(TargetTriple(host.to_string())); + } + } + + eyre::bail!("could not parse host target triple from `rustc -vV`") +} + +/// Parse an explicit Cargo `--target ` / `--target=` flag from +/// forwarded args, considering only arguments **before** `--`. +/// +/// Arguments after `--` belong to the test binary or run target and must not +/// affect cargo-fc target planning, e.g. `cargo fc run -- --target value`. +#[must_use] +pub fn parse_cli_target(cargo_args: &[String]) -> Option { + let mut it = cargo_args.iter(); + while let Some(arg) = it.next() { + if arg == "--" { + break; + } + if arg == "--target" + && let Some(v) = it.next() + { + return Some(v.clone()); + } + if let Some(v) = arg.strip_prefix("--target=") + && !v.is_empty() + { + return Some(v.to_string()); + } + } + None +} + /// Read-only access to environment variables. /// /// This trait abstracts environment variable lookups so that production code @@ -66,46 +221,8 @@ impl RustcTargetDetector { Self { env } } - fn host_triple() -> eyre::Result { - let output = Command::new("rustc") - .arg("-vV") - .output() - .wrap_err("failed to invoke rustc to detect host target")?; - - if !output.status.success() { - eyre::bail!("rustc -vV failed with exit code {:?}", output.status.code()); - } - - let stdout = String::from_utf8_lossy(&output.stdout); - for line in stdout.lines() { - if let Some(host) = line.strip_prefix("host: ") { - let host = host.trim(); - if host.is_empty() { - continue; - } - return Ok(TargetTriple(host.to_string())); - } - } - - eyre::bail!("could not parse host target triple from `rustc -vV`") - } - fn parse_target_flag(cargo_args: &[String]) -> Option { - // Support both `--target x` and `--target=x`. - let mut it = cargo_args.iter(); - while let Some(arg) = it.next() { - if arg == "--target" - && let Some(v) = it.next() - { - return Some(v.clone()); - } - if let Some(v) = arg.strip_prefix("--target=") - && !v.is_empty() - { - return Some(v.to_string()); - } - } - None + parse_cli_target(cargo_args) } } @@ -120,16 +237,66 @@ impl TargetDetector for RustcTargetDetector { return Ok(TargetTriple(triple.to_string())); } } - Self::host_triple() + host_triple() } } #[cfg(test)] mod test { - use super::{Env, RustcTargetDetector, TargetDetector}; + use super::{Env, RustcTargetDetector, TargetDetector, parse_cli_target}; use color_eyre::eyre; use std::collections::HashMap; + fn owned(values: &[&str]) -> Vec { + values.iter().copied().map(String::from).collect() + } + + #[test] + fn parse_cli_target_separate_value() { + assert_eq!( + parse_cli_target(&owned(&["check", "--target", "wasm32-unknown-unknown"])), + Some("wasm32-unknown-unknown".to_string()) + ); + } + + #[test] + fn parse_cli_target_equals_form() { + assert_eq!( + parse_cli_target(&owned(&["check", "--target=wasm32-unknown-unknown"])), + Some("wasm32-unknown-unknown".to_string()) + ); + } + + #[test] + fn parse_cli_target_ignores_value_after_double_dash() { + // `cargo fc run -- --target value-for-the-binary` must not be treated as + // cargo's target triple. + assert_eq!( + parse_cli_target(&owned(&["run", "--", "--target", "binary-arg"])), + None + ); + } + + #[test] + fn parse_cli_target_before_double_dash_still_parsed() { + assert_eq!( + parse_cli_target(&owned(&[ + "run", + "--target", + "x86_64-unknown-linux-gnu", + "--", + "--target", + "binary-arg", + ])), + Some("x86_64-unknown-linux-gnu".to_string()) + ); + } + + #[test] + fn parse_cli_target_absent() { + assert_eq!(parse_cli_target(&owned(&["check", "--all-features"])), None); + } + #[derive(Default)] struct TestEnv { vars: HashMap, @@ -188,6 +355,26 @@ mod test { Ok(()) } + #[test] + fn detector_ignores_target_flag_after_double_dash() -> eyre::Result<()> { + let env = TestEnv { + vars: HashMap::from([( + "CARGO_BUILD_TARGET".to_string(), + "aarch64-unknown-linux-gnu".to_string(), + )]), + }; + let d = RustcTargetDetector::with_env(env); + let args = vec![ + "run".to_string(), + "--".to_string(), + "--target".to_string(), + "binary-arg".to_string(), + ]; + let triple = d.detect_target(&args)?; + assert_eq!(triple.as_str(), "aarch64-unknown-linux-gnu"); + Ok(()) + } + #[test] fn empty_env_var_falls_through_to_host() -> eyre::Result<()> { let env = TestEnv { diff --git a/src/target_plan.rs b/src/target_plan.rs new file mode 100644 index 0000000..f013579 --- /dev/null +++ b/src/target_plan.rs @@ -0,0 +1,891 @@ +//! Target selection and target-plan construction. +//! +//! This is the outer execution axis: it decides which target triples each +//! selected package is visited for, deduplicated by triple for stable +//! scheduling and output, while every package-target assignment carries its +//! [`TargetSource`] so injection and output decisions stay local to the +//! assignment. +//! +//! Target *selection* (this module) is kept separate from target-specific +//! *config resolution* ([`crate::config::resolve`]): target lists choose the +//! outer axis; `target.'cfg(...)'` overrides shape the effective feature matrix +//! for one already-selected target. + +use crate::cfg_eval::CfgEvaluator; +use crate::config::{Config, WorkspaceConfig, WorkspaceTargetOverride}; +use crate::target::{EffectiveTarget, TargetEnvironment, TargetSource, TargetTriple}; +use color_eyre::eyre::{self, WrapErr}; +use std::collections::{BTreeMap, HashSet}; + +/// A package selected for processing together with its cached base config. +/// +/// Configs are loaded once before planning so neither planning nor +/// execution-plan construction re-reads the manifest (which would duplicate +/// deprecation warnings). +pub struct SelectedPackage<'a> { + /// The selected package. + pub package: &'a cargo_metadata::Package, + /// The cached base cargo-fc config for this package. + pub config: &'a Config, +} + +/// A package assigned to one concrete target. +pub struct PlannedPackage<'a> { + /// The package. + pub package: &'a cargo_metadata::Package, + /// The cached base cargo-fc config for this package. + pub config: &'a Config, + /// The concrete target and where it came from. + pub target: EffectiveTarget, +} + +/// All package assignments for one concrete target triple. +pub struct TargetPlan<'a> { + /// The concrete target triple this plan is for. + pub target: TargetTriple, + /// The package assignments for this target, in selected-package order. + pub packages: Vec>, +} + +/// The full set of target plans for an invocation. +pub struct TargetPlans<'a> { + /// Target plans in deterministic order (workspace target order, then + /// package-only targets, then the fallback target). + pub plans: Vec>, + /// Whether target selection was influenced by configured target metadata + /// or an explicit `--target` (anything other than the implicit + /// host/`CARGO_BUILD_TARGET` single-target fallback). + /// + /// Includes the package `targets = []` opt-out, even if the resulting + /// concrete source is `Host`/`CargoBuildTargetEnv`. Its only consumer is + /// output formatting: it gates whether per-entry summaries show the + /// `target = ...` column. + pub contains_configured_assignments: bool, +} + +impl TargetPlans<'_> { + /// The distinct target triples planned, in plan order. + #[must_use] + pub fn distinct_targets(&self) -> Vec { + self.plans.iter().map(|p| p.target.clone()).collect() + } +} + +/// Trim, reject empty, and deduplicate a configured target list, preserving +/// first-occurrence order. +fn normalize_targets(raw: &[String]) -> eyre::Result> { + let mut seen = HashSet::new(); + let mut out = Vec::new(); + for triple in raw { + let triple = triple.trim(); + if triple.is_empty() { + eyre::bail!("empty target triple in configured `targets` list"); + } + if seen.insert(triple.to_string()) { + out.push(TargetTriple(triple.to_string())); + } + } + Ok(out) +} + +/// Resolve the single fallback target: `CARGO_BUILD_TARGET`, then host. +fn resolve_fallback( + env: &impl TargetEnvironment, + cache: &mut Option, +) -> eyre::Result { + if let Some(target) = cache { + return Ok(target.clone()); + } + let target = if let Some(triple) = env.cargo_build_target() { + EffectiveTarget { + triple: TargetTriple(triple), + source: TargetSource::CargoBuildTargetEnv, + } + } else { + EffectiveTarget { + triple: env.host_target()?, + source: TargetSource::Host, + } + }; + *cache = Some(target.clone()); + Ok(target) +} + +/// Per-package resolved effective target list. +struct PackageTargets<'a> { + package: &'a cargo_metadata::Package, + config: &'a Config, + targets: Vec, +} + +/// Resolve one selected package's effective target list using the configured +/// precedence (CLI handled by the caller as a global override). +fn package_target_list( + selected: &SelectedPackage<'_>, + workspace_targets: &[TargetTriple], + env: &impl TargetEnvironment, + fallback_cache: &mut Option, +) -> eyre::Result> { + match &selected.config.targets { + // Package-level list present. + Some(list) if !list.is_empty() => { + let triples = normalize_targets(list)?; + Ok(triples + .into_iter() + .map(|triple| EffectiveTarget { + triple, + source: TargetSource::PackageConfig, + }) + .collect()) + } + // Package-level opt-out (`targets = []`): use the fallback single target. + Some(_) => Ok(vec![resolve_fallback(env, fallback_cache)?]), + // No package-level list: inherit workspace targets, else fallback. + None => { + if workspace_targets.is_empty() { + Ok(vec![resolve_fallback(env, fallback_cache)?]) + } else { + Ok(workspace_targets + .iter() + .map(|triple| EffectiveTarget { + triple: triple.clone(), + source: TargetSource::WorkspaceConfig, + }) + .collect()) + } + } + } +} + +/// Resolve the effective workspace `exclude_packages` set for one target. +/// +/// Starts from the base (target-independent) exclude set and applies matching +/// workspace `target.'cfg(...)'` `exclude_packages` patches deterministically +/// (cfg key order). Uses the same patch semantics as package target overrides. +fn resolve_target_excludes( + base: &HashSet, + overrides: &BTreeMap, + triple: &TargetTriple, + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result> { + let mut any = false; + let mut override_value: Option> = None; + let mut add: HashSet = HashSet::new(); + let mut remove: HashSet = HashSet::new(); + + for (expr, ov) in overrides { + let is_match = evaluator + .matches(expr, triple) + .wrap_err_with(|| format!("failed to evaluate cfg expression `{expr}`"))?; + if !is_match { + continue; + } + let Some(patch) = &ov.exclude_packages else { + continue; + }; + any = true; + + if let Some(ovv) = patch.override_value() { + match &override_value { + None => override_value = Some(ovv.clone()), + Some(existing) => { + if existing != ovv { + eyre::bail!( + "conflicting overrides for `exclude_packages` from workspace target override `{expr}`" + ); + } + } + } + } + add.extend(patch.add_values().iter().cloned()); + remove.extend(patch.remove_values().iter().cloned()); + } + + if !any { + return Ok(base.clone()); + } + + // Order: start from override (or base), then remove, then add (add wins). + let mut out = override_value.unwrap_or_else(|| base.clone()); + for r in &remove { + out.remove(r); + } + out.extend(add); + Ok(out) +} + +/// Build the target plans for an invocation. +/// +/// When `capability_allowed` is false, configured workspace/package target +/// lists are ignored and planning falls back to the single effective target +/// (`--target`, then `CARGO_BUILD_TARGET`, then host). Workspace target +/// overrides (`exclude_packages` patches) still apply to every concrete target, +/// including single-target invocations. +/// +/// # Errors +/// +/// Returns an error if a target list contains an empty triple, if workspace +/// target overrides conflict, or if cfg evaluation fails. +#[allow( + clippy::implicit_hasher, + reason = "callers always pass the std default-hasher HashSet from base_workspace_exclude_packages" +)] +pub fn build_target_plans<'a>( + selected: &[SelectedPackage<'a>], + workspace_config: &WorkspaceConfig, + base_exclude: &HashSet, + cli_target: Option<&str>, + capability_allowed: bool, + env: &impl TargetEnvironment, + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result> { + let mut fallback_cache: Option = None; + + let contains_configured_assignments = if cli_target.is_some() { + true + } else if capability_allowed { + !workspace_config.targets.is_empty() || selected.iter().any(|s| s.config.targets.is_some()) + } else { + false + }; + + // Resolve each selected package's effective target list. + let package_targets: Vec> = if let Some(cli) = cli_target { + // Explicit `--target` wins globally: every package runs the single CLI + // target and all configured lists are ignored. Cargo already received + // the flag from the user, so the source is `Cli` (no injection). + let triple = cli.trim(); + if triple.is_empty() { + eyre::bail!("empty `--target` value"); + } + let cli_target = EffectiveTarget { + triple: TargetTriple(triple.to_string()), + source: TargetSource::Cli, + }; + selected + .iter() + .map(|s| PackageTargets { + package: s.package, + config: s.config, + targets: vec![cli_target.clone()], + }) + .collect() + } else if capability_allowed { + let workspace_targets = normalize_targets(&workspace_config.targets)?; + let mut out = Vec::with_capacity(selected.len()); + for s in selected { + let targets = package_target_list(s, &workspace_targets, env, &mut fallback_cache)?; + out.push(PackageTargets { + package: s.package, + config: s.config, + targets, + }); + } + out + } else { + // Capability denied: ignore configured lists, use the fallback single + // target for every package. + let fallback = resolve_fallback(env, &mut fallback_cache)?; + selected + .iter() + .map(|s| PackageTargets { + package: s.package, + config: s.config, + targets: vec![fallback.clone()], + }) + .collect() + }; + + // Build the global target order: workspace target order first, then + // package-only targets in selected-package order, then the fallback target + // (which appears via the packages that use it). + let mut order: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + + if cli_target.is_none() && capability_allowed { + for triple in normalize_targets(&workspace_config.targets)? { + if seen.insert(triple.clone()) { + order.push(triple); + } + } + } + for pt in &package_targets { + for et in &pt.targets { + if seen.insert(et.triple.clone()) { + order.push(et.triple.clone()); + } + } + } + + // For each target in order, attach packages whose effective list contains + // it (preserving each package's source), apply the effective workspace + // exclude set for that target, and drop empty plans. + let mut plans = Vec::new(); + for triple in &order { + let exclude = + resolve_target_excludes(base_exclude, &workspace_config.target, triple, evaluator)?; + let mut packages = Vec::new(); + for pt in &package_targets { + if exclude.contains(pt.package.name.as_str()) { + continue; + } + if let Some(et) = pt.targets.iter().find(|et| &et.triple == triple) { + packages.push(PlannedPackage { + package: pt.package, + config: pt.config, + target: et.clone(), + }); + } + } + if !packages.is_empty() { + plans.push(TargetPlan { + target: triple.clone(), + packages, + }); + } + } + + Ok(TargetPlans { + plans, + contains_configured_assignments, + }) +} + +#[cfg(test)] +mod test { + use super::{SelectedPackage, build_target_plans}; + use crate::cfg_eval::CfgEvaluator; + use crate::config::patch::StringSetPatch; + use crate::config::{Config, WorkspaceConfig, WorkspaceTargetOverride}; + use crate::target::{TargetEnvironment, TargetSource, TargetTriple}; + use color_eyre::eyre; + use std::collections::{BTreeMap, HashSet}; + + struct TestEnv { + build_target: Option, + host: String, + } + + impl TestEnv { + fn host(host: &str) -> Self { + Self { + build_target: None, + host: host.to_string(), + } + } + } + + impl TargetEnvironment for TestEnv { + fn cargo_build_target(&self) -> Option { + self.build_target.clone() + } + fn host_target(&self) -> eyre::Result { + Ok(TargetTriple(self.host.clone())) + } + } + + #[derive(Default)] + struct StubEval { + matches: HashSet, + } + + impl CfgEvaluator for StubEval { + fn matches(&mut self, cfg_expr: &str, _target: &TargetTriple) -> eyre::Result { + Ok(self.matches.contains(cfg_expr)) + } + } + + /// Evaluator that matches a different cfg per target triple, for the + /// target-specific workspace exclude test. + struct PerTargetEval; + + impl CfgEvaluator for PerTargetEval { + fn matches(&mut self, cfg_expr: &str, target: &TargetTriple) -> eyre::Result { + Ok(match target.0.as_str() { + "linux" => cfg_expr == "cfg(target_os = \"linux\")", + "wasm" => cfg_expr == "cfg(target_arch = \"wasm32\")", + _ => false, + }) + } + } + + fn package(name: &str) -> eyre::Result { + use cargo_metadata::{PackageBuilder, PackageId, PackageName}; + use semver::Version; + use std::str::FromStr as _; + Ok(PackageBuilder::new( + PackageName::from_str(name)?, + Version::parse("0.1.0")?, + PackageId { + repr: name.to_string(), + }, + "", + ) + .build()?) + } + + fn config_with_targets(targets: Option<&[&str]>) -> Config { + Config { + targets: targets.map(|t| t.iter().map(|s| (*s).to_string()).collect()), + ..Config::default() + } + } + + fn workspace_targets(targets: &[&str]) -> WorkspaceConfig { + WorkspaceConfig { + targets: targets.iter().map(|s| (*s).to_string()).collect(), + ..WorkspaceConfig::default() + } + } + + fn triples(plan_targets: &[&super::TargetPlan<'_>]) -> Vec { + plan_targets.iter().map(|p| p.target.0.clone()).collect() + } + + #[test] + fn no_config_falls_back_to_host_single_target() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = Config::default(); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = WorkspaceConfig::default(); + let env = TestEnv::host("host-triple"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert!(!plans.contains_configured_assignments); + assert_eq!(plans.plans.len(), 1); + let plan = &plans.plans[0]; + assert_eq!(plan.target.0, "host-triple"); + assert_eq!(plan.packages.len(), 1); + assert_eq!(plan.packages[0].target.source, TargetSource::Host); + Ok(()) + } + + #[test] + fn workspace_targets_expand_in_order() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = Config::default(); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = workspace_targets(&["linux", "windows"]); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert!(plans.contains_configured_assignments); + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec!["linux".to_string(), "windows".to_string()] + ); + for plan in &plans.plans { + assert_eq!( + plan.packages[0].target.source, + TargetSource::WorkspaceConfig + ); + } + Ok(()) + } + + #[test] + fn package_targets_override_workspace_and_keep_order_after() -> eyre::Result<()> { + // `web` opts into wasm only; `core` inherits the workspace list. + let web = package("web")?; + let core = package("core")?; + let web_cfg = config_with_targets(Some(&["wasm32-unknown-unknown"])); + let core_cfg = config_with_targets(None); + let selected = vec![ + SelectedPackage { + package: &web, + config: &web_cfg, + }, + SelectedPackage { + package: &core, + config: &core_cfg, + }, + ]; + let ws = workspace_targets(&["linux", "windows"]); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + // Workspace targets first, then the package-only wasm target. + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec![ + "linux".to_string(), + "windows".to_string(), + "wasm32-unknown-unknown".to_string() + ] + ); + + let names = |plan: &super::TargetPlan<'_>| { + plan.packages + .iter() + .map(|p| p.package.name.to_string()) + .collect::>() + }; + assert_eq!(names(&plans.plans[0]), vec!["core".to_string()]); + assert_eq!(names(&plans.plans[1]), vec!["core".to_string()]); + assert_eq!(names(&plans.plans[2]), vec!["web".to_string()]); + Ok(()) + } + + #[test] + fn package_opt_out_uses_fallback() -> eyre::Result<()> { + let pkg = package("native")?; + let cfg = config_with_targets(Some(&[])); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = workspace_targets(&["linux"]); + let env = TestEnv::host("host-triple"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + // Opt-out package uses the host fallback, not the workspace list. But + // because a configured list exists in the workspace, the host target + // plan is still produced for this package only. + assert!(plans.contains_configured_assignments); + // The workspace `linux` target has no participating package (the only + // package opted out), so it is dropped. + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec!["host-triple".to_string()] + ); + assert_eq!(plans.plans[0].packages[0].target.source, TargetSource::Host); + Ok(()) + } + + #[test] + fn explicit_cli_target_overrides_everything() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = config_with_targets(Some(&["wasm32-unknown-unknown"])); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = workspace_targets(&["linux", "windows"]); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let plans = build_target_plans( + &selected, + &ws, + &HashSet::new(), + Some("aarch64-apple-darwin"), + true, + &env, + &mut eval, + )?; + + assert!(plans.contains_configured_assignments); + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec!["aarch64-apple-darwin".to_string()] + ); + assert_eq!(plans.plans[0].packages[0].target.source, TargetSource::Cli); + Ok(()) + } + + #[test] + fn duplicate_targets_deduped_preserving_order() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = config_with_targets(Some(&["linux", "windows", "linux"])); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = WorkspaceConfig::default(); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec!["linux".to_string(), "windows".to_string()] + ); + Ok(()) + } + + #[test] + fn same_triple_keeps_distinct_sources_per_package() -> eyre::Result<()> { + // `a` inherits the workspace `linux`; `b` lists `linux` itself. + let a = package("a")?; + let b = package("b")?; + let a_cfg = config_with_targets(None); + let b_cfg = config_with_targets(Some(&["linux"])); + let selected = vec![ + SelectedPackage { + package: &a, + config: &a_cfg, + }, + SelectedPackage { + package: &b, + config: &b_cfg, + }, + ]; + let ws = workspace_targets(&["linux"]); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert_eq!(plans.plans.len(), 1); + let plan = &plans.plans[0]; + assert_eq!(plan.target.0, "linux"); + let by_name = |name: &str| { + plan.packages + .iter() + .find(|p| p.package.name.as_str() == name) + .map(|p| p.target.source) + }; + assert_eq!(by_name("a"), Some(TargetSource::WorkspaceConfig)); + assert_eq!(by_name("b"), Some(TargetSource::PackageConfig)); + Ok(()) + } + + #[test] + fn capability_denied_ignores_configured_lists() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = config_with_targets(Some(&["wasm32-unknown-unknown"])); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = workspace_targets(&["linux", "windows"]); + let env = TestEnv::host("host-triple"); + let mut eval = StubEval::default(); + + let plans = build_target_plans( + &selected, + &ws, + &HashSet::new(), + None, + false, + &env, + &mut eval, + )?; + + assert!(!plans.contains_configured_assignments); + assert_eq!( + triples(&plans.plans.iter().collect::>()), + vec!["host-triple".to_string()] + ); + assert_eq!(plans.plans[0].packages[0].target.source, TargetSource::Host); + Ok(()) + } + + #[test] + fn cargo_build_target_used_as_fallback_source() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = Config::default(); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = WorkspaceConfig::default(); + let env = TestEnv { + build_target: Some("aarch64-unknown-linux-gnu".to_string()), + host: "host".to_string(), + }; + let mut eval = StubEval::default(); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert!(!plans.contains_configured_assignments); + assert_eq!(plans.plans[0].target.0, "aarch64-unknown-linux-gnu"); + assert_eq!( + plans.plans[0].packages[0].target.source, + TargetSource::CargoBuildTargetEnv + ); + Ok(()) + } + + #[test] + fn workspace_target_override_excludes_only_matching_targets() -> eyre::Result<()> { + let native = package("native-cli")?; + let wasm_app = package("wasm-app")?; + let native_cfg = Config::default(); + let wasm_cfg = Config::default(); + let selected = vec![ + SelectedPackage { + package: &native, + config: &native_cfg, + }, + SelectedPackage { + package: &wasm_app, + config: &wasm_cfg, + }, + ]; + + let mut target = BTreeMap::new(); + target.insert( + "cfg(target_arch = \"wasm32\")".to_string(), + WorkspaceTargetOverride { + exclude_packages: Some(StringSetPatch::Patch { + r#override: None, + add: HashSet::from(["native-cli".to_string()]), + remove: HashSet::new(), + }), + }, + ); + target.insert( + "cfg(target_os = \"linux\")".to_string(), + WorkspaceTargetOverride { + exclude_packages: Some(StringSetPatch::Patch { + r#override: None, + add: HashSet::from(["wasm-app".to_string()]), + remove: HashSet::new(), + }), + }, + ); + let ws = WorkspaceConfig { + targets: vec!["linux".to_string(), "wasm".to_string()], + target, + ..WorkspaceConfig::default() + }; + + let env = TestEnv::host("host"); + + // The exclude set is target-specific, so use an evaluator that matches a + // different cfg per target triple. + let mut eval = PerTargetEval; + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert_eq!(plans.plans.len(), 2); + let names = |plan: &super::TargetPlan<'_>| { + plan.packages + .iter() + .map(|p| p.package.name.to_string()) + .collect::>() + }; + assert_eq!(plans.plans[0].target.0, "linux"); + assert_eq!(names(&plans.plans[0]), vec!["native-cli".to_string()]); + assert_eq!(plans.plans[1].target.0, "wasm"); + assert_eq!(names(&plans.plans[1]), vec!["wasm-app".to_string()]); + Ok(()) + } + + #[test] + fn workspace_target_override_applies_to_single_target() -> eyre::Result<()> { + // No configured target list, single host target, but a workspace target + // override still excludes a package for the matching host cfg. + let keep = package("keep")?; + let drop = package("drop")?; + let keep_cfg = Config::default(); + let drop_cfg = Config::default(); + let selected = vec![ + SelectedPackage { + package: &keep, + config: &keep_cfg, + }, + SelectedPackage { + package: &drop, + config: &drop_cfg, + }, + ]; + + let mut target = BTreeMap::new(); + target.insert( + "cfg(target_os = \"linux\")".to_string(), + WorkspaceTargetOverride { + exclude_packages: Some(StringSetPatch::Override(HashSet::from([ + "drop".to_string() + ]))), + }, + ); + let ws = WorkspaceConfig { + target, + ..WorkspaceConfig::default() + }; + + let env = TestEnv::host("x86_64-unknown-linux-gnu"); + let mut eval = StubEval::default(); + eval.matches + .insert("cfg(target_os = \"linux\")".to_string()); + + let plans = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval)?; + + assert_eq!(plans.plans.len(), 1); + let names: Vec = plans.plans[0] + .packages + .iter() + .map(|p| p.package.name.to_string()) + .collect(); + assert_eq!(names, vec!["keep".to_string()]); + Ok(()) + } + + #[test] + fn base_exclude_applies_to_all_targets() -> eyre::Result<()> { + let keep = package("keep")?; + let drop = package("drop")?; + let keep_cfg = Config::default(); + let drop_cfg = Config::default(); + let selected = vec![ + SelectedPackage { + package: &keep, + config: &keep_cfg, + }, + SelectedPackage { + package: &drop, + config: &drop_cfg, + }, + ]; + let ws = workspace_targets(&["linux", "windows"]); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + let base_exclude = HashSet::from(["drop".to_string()]); + + let plans = build_target_plans(&selected, &ws, &base_exclude, None, true, &env, &mut eval)?; + + for plan in &plans.plans { + let names: Vec = plan + .packages + .iter() + .map(|p| p.package.name.to_string()) + .collect(); + assert_eq!(names, vec!["keep".to_string()]); + } + Ok(()) + } + + #[test] + fn empty_triple_in_list_is_rejected() -> eyre::Result<()> { + let pkg = package("a")?; + let cfg = config_with_targets(Some(&["linux", " "])); + let selected = vec![SelectedPackage { + package: &pkg, + config: &cfg, + }]; + let ws = WorkspaceConfig::default(); + let env = TestEnv::host("host"); + let mut eval = StubEval::default(); + + let result = + build_target_plans(&selected, &ws, &HashSet::new(), None, true, &env, &mut eval); + assert!(result.is_err()); + Ok(()) + } +} diff --git a/src/workspace.rs b/src/workspace.rs index 1cbe211..7a01b65 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -1,13 +1,16 @@ //! Workspace-level configuration and package discovery. use crate::config::{Config, WorkspaceConfig}; -use crate::package::Package; use crate::print_warning; use crate::{ DEFAULT_METADATA_KEY, METADATA_KEYS, find_metadata_value, pkg_metadata_section, ws_metadata_section, }; use color_eyre::eyre; +use std::collections::HashSet; + +/// Workspace-only metadata keys read solely from the workspace root. +const WORKSPACE_ONLY_KEYS: &[&str] = &["exclude_packages", "targets", "target", "subcommands"]; /// Abstraction over a Cargo workspace used by this crate. pub trait Workspace { @@ -19,8 +22,35 @@ pub trait Workspace { /// deserialized. fn workspace_config(&self) -> eyre::Result; + /// Return the candidate packages for feature combinations **without** + /// applying workspace package exclusions. + /// + /// This emits the deprecation and no-op warnings for misplaced workspace + /// metadata once. Workspace `exclude_packages` (and its target-specific + /// patches) are applied later, per target, by the planner. + /// + /// # Errors + /// + /// Returns an error if metadata can not be parsed. + fn candidate_packages_for_fc(&self) -> eyre::Result>; + + /// Return the base, target-independent workspace exclude set. + /// + /// This is the union of `[workspace.metadata.*].exclude_packages` and the + /// deprecated root-package `exclude_packages`. Target-specific workspace + /// overrides patch this set per target during planning. This method emits + /// no warnings. + /// + /// # Errors + /// + /// Returns an error if workspace metadata can not be parsed. + fn base_workspace_exclude_packages(&self) -> eyre::Result>; + /// Return the packages that should be considered for feature combinations. /// + /// This is the backward-compatible single path: candidate discovery plus + /// the base (target-independent) workspace exclude set. + /// /// # Errors /// /// Returns an error if per-package configuration can not be parsed. @@ -36,91 +66,105 @@ impl Workspace for cargo_metadata::Metadata { Ok(config) } + fn candidate_packages_for_fc(&self) -> eyre::Result> { + warn_workspace_metadata_misuse(self); + Ok(self.workspace_packages()) + } + + fn base_workspace_exclude_packages(&self) -> eyre::Result> { + let mut exclude = self.workspace_config()?.exclude_packages; + + // Fold in the deprecated root-package exclude_packages without emitting + // warnings here (warnings are emitted once in candidate discovery). + if let Some(root_package) = self.root_package() + && let Some((value, _key)) = find_metadata_value(&root_package.metadata) + && let Ok(config) = serde_json::from_value::(value.clone()) + { + exclude.extend(config.exclude_packages); + } + + Ok(exclude) + } + fn packages_for_fc(&self) -> eyre::Result> { - let mut packages = self.workspace_packages(); - - let workspace_config = self.workspace_config()?; - - // Determine the workspace root package (if any) and load its config so we can both - // apply filtering and emit deprecation warnings for legacy configuration. - let mut root_config: Option = None; - let mut root_id: Option = None; - - if let Some(root_package) = self.root_package() { - let root_key = find_metadata_value(&root_package.metadata) - .map_or(DEFAULT_METADATA_KEY, |(_, key)| key); - let config = root_package.config()?; - - if !config.exclude_packages.is_empty() { - print_warning!( - "{}.exclude_packages in the workspace root package is deprecated; use {}.exclude_packages instead", - pkg_metadata_section(root_key), - ws_metadata_section(root_key), - ); - } + let mut packages = self.candidate_packages_for_fc()?; + let exclude = self.base_workspace_exclude_packages()?; + packages.retain(|p| !exclude.contains(p.name.as_str())); + Ok(packages) + } +} - root_id = Some(root_package.id.clone()); - root_config = Some(config); +/// Emit deprecation and no-op warnings for misplaced workspace metadata. +/// +/// Warnings are intentionally side effects of candidate discovery so they fire +/// once per invocation regardless of how many targets are later planned. +fn warn_workspace_metadata_misuse(metadata: &cargo_metadata::Metadata) { + let Some(root_package) = metadata.root_package() else { + return; + }; + + let root_key = + find_metadata_value(&root_package.metadata).map_or(DEFAULT_METADATA_KEY, |(_, key)| key); + + // Root-package exclude_packages is deprecated in favor of workspace metadata. + if let Some((value, _key)) = find_metadata_value(&root_package.metadata) + && let Ok(config) = serde_json::from_value::(value.clone()) + && !config.exclude_packages.is_empty() + { + print_warning!( + "{}.exclude_packages in the workspace root package is deprecated; use {}.exclude_packages instead", + pkg_metadata_section(root_key), + ws_metadata_section(root_key), + ); + } + + let root_id = &root_package.id; + for package in &metadata.packages { + if &package.id == root_id { + continue; } - // For non-root workspace members, using exclude_packages is a no-op. Emit warnings for - // such configurations so users are aware that these fields are ignored. - if root_id.is_some() { - for package in &self.packages { - if Some(&package.id) == root_id.as_ref() { - continue; - } + // [package.metadata.].exclude_packages in a non-root member is a no-op. + if let Some((raw, key)) = find_metadata_value(&package.metadata) + && let Ok(config) = serde_json::from_value::(raw.clone()) + && !config.exclude_packages.is_empty() + { + print_warning!( + "{}.exclude_packages in package `{}` has no effect; this field is only read from the workspace root Cargo.toml", + pkg_metadata_section(key), + package.name, + ); + } - // [package.metadata.].exclude_packages - if let Some((raw, key)) = find_metadata_value(&package.metadata) - && let Ok(config) = serde_json::from_value::(raw.clone()) - && !config.exclude_packages.is_empty() - { + // [workspace.metadata.]. specified in non-root manifests is + // also a no-op. Detect the JSON shape produced by cargo metadata and + // warn for any workspace-only key that carries values. + if let Some(workspace) = package.metadata.get("workspace") + && let Some((key, tool)) = METADATA_KEYS + .iter() + .find_map(|&key| workspace.get(key).map(|tool| (key, tool))) + { + for ws_key in WORKSPACE_ONLY_KEYS { + if json_has_values(tool.get(*ws_key)) { print_warning!( - "{}.exclude_packages in package `{}` has no effect; this field is only read from the workspace root Cargo.toml", - pkg_metadata_section(key), + "{}.{} in package `{}` has no effect; workspace metadata is only read from the workspace root Cargo.toml", + ws_metadata_section(key), + ws_key, package.name, ); } - - // [workspace.metadata.].exclude_packages specified in - // non-root manifests is also a no-op. Detect the likely JSON shape produced by - // cargo metadata and warn if present. - if let Some(workspace) = package.metadata.get("workspace") { - let ws_tool = METADATA_KEYS - .iter() - .find_map(|&key| workspace.get(key).map(|tool| (key, tool))); - - if let Some((key, tool)) = ws_tool - && let Some(exclude_packages) = tool.get("exclude_packages") - { - let has_values = match exclude_packages { - serde_json::Value::Array(values) => !values.is_empty(), - serde_json::Value::Null => false, - _ => true, - }; - - if has_values { - print_warning!( - "{}.exclude_packages in package `{}` has no effect; workspace metadata is only read from the workspace root Cargo.toml", - ws_metadata_section(key), - package.name, - ); - } - } - } } } + } +} - // Filter packages based on workspace metadata configuration - packages.retain(|p| !workspace_config.exclude_packages.contains(p.name.as_str())); - - if let Some(config) = root_config { - // Filter packages based on root package Cargo.toml configuration - packages.retain(|p| !config.exclude_packages.contains(p.name.as_str())); - } - - Ok(packages) +/// Whether a JSON value carries meaningful (non-empty) configuration. +fn json_has_values(value: Option<&serde_json::Value>) -> bool { + match value { + Some(serde_json::Value::Array(values)) => !values.is_empty(), + Some(serde_json::Value::Object(map)) => !map.is_empty(), + Some(serde_json::Value::Null) | None => false, + Some(_) => true, } } diff --git a/tests/per_target.rs b/tests/per_target.rs new file mode 100644 index 0000000..5275ef3 --- /dev/null +++ b/tests/per_target.rs @@ -0,0 +1,498 @@ +//! Integration tests for the per-target feature-combination axis. +//! +//! These drive the public planning + matrix API end to end over real +//! `cargo metadata`, asserting on the matrix rows and target plans. They use +//! deterministic stub adapters (a fixed host environment and a cfg evaluator) +//! so no real `rustc`/`cargo` invocation or installed target is required. + +use assert_fs::TempDir; +use assert_fs::prelude::*; +use cargo_feature_combinations::Package as _; +use cargo_feature_combinations::cfg_eval::CfgEvaluator; +use cargo_feature_combinations::cli::Options; +use cargo_feature_combinations::config::Config; +use cargo_feature_combinations::runner::{build_execution_plans, build_matrix_rows}; +use cargo_feature_combinations::target::{TargetEnvironment, TargetTriple}; +use cargo_feature_combinations::target_plan::{SelectedPackage, build_target_plans}; +use cargo_feature_combinations::workspace::Workspace as _; +use color_eyre::eyre::{self, OptionExt}; +use std::collections::HashSet; + +struct HostEnv(&'static str); + +impl TargetEnvironment for HostEnv { + fn cargo_build_target(&self) -> Option { + None + } + fn host_target(&self) -> eyre::Result { + Ok(TargetTriple(self.0.to_string())) + } +} + +/// cfg evaluator matching `(triple, cfg)` pairs exactly, with no real `rustc`. +#[derive(Default)] +struct PairEval { + rules: Vec<(String, String)>, +} + +impl PairEval { + fn matching(triple: &str, cfg: &str) -> Self { + Self { + rules: vec![(triple.to_string(), cfg.to_string())], + } + } +} + +impl CfgEvaluator for PairEval { + fn matches(&mut self, cfg_expr: &str, target: &TargetTriple) -> eyre::Result { + Ok(self + .rules + .iter() + .any(|(t, c)| t == target.as_str() && c == cfg_expr)) + } +} + +fn single_crate(cargo_toml: &str) -> eyre::Result { + let temp = TempDir::new()?; + temp.child("Cargo.toml").write_str(cargo_toml)?; + temp.child("src/lib.rs").write_str("pub fn x() {}\n")?; + Ok(temp) +} + +fn metadata(temp: &TempDir) -> eyre::Result { + Ok(cargo_metadata::MetadataCommand::new() + .current_dir(temp.path()) + .no_deps() + .exec()?) +} + +/// Build matrix rows for a metadata workspace using the deterministic adapters. +fn matrix_rows( + meta: &cargo_metadata::Metadata, + cli_target: Option<&str>, + capability_allowed: bool, + env: &impl TargetEnvironment, + evaluator: &mut impl CfgEvaluator, +) -> eyre::Result> { + let ws_config = meta.workspace_config()?; + let packages = meta.candidate_packages_for_fc()?; + let configs: Vec = packages + .iter() + .map(|p| p.config()) + .collect::>>()?; + let selected: Vec> = packages + .iter() + .zip(&configs) + .map(|(package, config)| SelectedPackage { package, config }) + .collect(); + let base_exclude = meta.base_workspace_exclude_packages()?; + + let target_plans = build_target_plans( + &selected, + &ws_config, + &base_exclude, + cli_target, + capability_allowed, + env, + evaluator, + )?; + let plan_set = build_execution_plans(&target_plans, &Options::default(), false, evaluator)?; + Ok(build_matrix_rows(&plan_set, false)) +} + +/// Reduce matrix rows to a comparable `(name, target, features)` set. +fn row_set(rows: &[serde_json::Value]) -> HashSet<(String, String, String)> { + rows.iter() + .map(|r| { + let s = |k: &str| { + r.get(k) + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .to_string() + }; + (s("name"), s("target"), s("features")) + }) + .collect() +} + +fn targets_in(rows: &[serde_json::Value]) -> HashSet { + rows.iter() + .filter_map(|r| r.get("target").and_then(serde_json::Value::as_str)) + .map(ToString::to_string) + .collect() +} + +#[test] +fn no_configured_targets_matrix_includes_host_on_every_row() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + b = [] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + assert!(!rows.is_empty()); + assert_eq!( + targets_in(&rows), + HashSet::from(["host-triple".to_string()]) + ); + Ok(()) +} + +#[test] +fn package_targets_multiply_matrix_rows() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + [package.metadata.cargo-fc] + targets = ["t-linux", "t-wasm"] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + // Two combos ([] and [a]) across two targets = 4 rows. + assert_eq!(rows.len(), 4); + assert_eq!( + targets_in(&rows), + HashSet::from(["t-linux".to_string(), "t-wasm".to_string()]) + ); + Ok(()) +} + +#[test] +fn explicit_cli_target_ignores_configured_lists() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + [package.metadata.cargo-fc] + targets = ["t-linux", "t-wasm"] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, Some("t-cli"), true, &env, &mut eval)?; + + assert_eq!(targets_in(&rows), HashSet::from(["t-cli".to_string()])); + Ok(()) +} + +#[test] +fn package_opt_out_uses_host_target() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + [package.metadata.cargo-fc] + targets = [] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + assert_eq!( + targets_in(&rows), + HashSet::from(["host-triple".to_string()]) + ); + Ok(()) +} + +#[test] +fn capability_denied_ignores_configured_targets() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + [package.metadata.cargo-fc] + targets = ["t-linux", "t-wasm"] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + // Capability denied → configured lists ignored, single host target. + let rows = matrix_rows(&meta, None, false, &env, &mut eval)?; + + assert_eq!( + targets_in(&rows), + HashSet::from(["host-triple".to_string()]) + ); + Ok(()) +} + +#[test] +fn target_override_changes_feature_rows_per_target() -> eyre::Result<()> { + // `a` is excluded only for `t-linux` via a matching cfg override, so the + // linux rows must not contain `a` while the wasm rows do. + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + b = [] + [package.metadata.cargo-fc] + targets = ["t-linux", "t-wasm"] + [package.metadata.cargo-fc.target.'cfg(target_os = "linux")'] + exclude_features = { add = ["a"] } + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::matching("t-linux", "cfg(target_os = \"linux\")"); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + let linux_has_a = rows.iter().any(|r| { + r.get("target").and_then(serde_json::Value::as_str) == Some("t-linux") + && r.get("features") + .and_then(serde_json::Value::as_str) + .is_some_and(|f| f.split(',').any(|x| x == "a")) + }); + let wasm_has_a = rows.iter().any(|r| { + r.get("target").and_then(serde_json::Value::as_str) == Some("t-wasm") + && r.get("features") + .and_then(serde_json::Value::as_str) + .is_some_and(|f| f.split(',').any(|x| x == "a")) + }); + assert!(!linux_has_a, "feature `a` should be excluded for t-linux"); + assert!(wasm_has_a, "feature `a` should remain for t-wasm"); + Ok(()) +} + +#[test] +fn matrix_reserved_target_key_is_overwritten_by_builtin() -> eyre::Result<()> { + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [package.metadata.cargo-fc] + targets = ["t-linux"] + [package.metadata.cargo-fc.matrix] + target = "user-supplied" + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + // The built-in target wins over the user-supplied matrix `target`. + assert_eq!(targets_in(&rows), HashSet::from(["t-linux".to_string()])); + Ok(()) +} + +fn workspace(root_toml: &str) -> eyre::Result { + let temp = TempDir::new()?; + temp.child("Cargo.toml").write_str(root_toml)?; + temp.child("member/Cargo.toml").write_str( + r#" + [package] + name = "member" + version = "0.1.0" + edition = "2021" + [features] + f = [] + "#, + )?; + temp.child("member/src/lib.rs") + .write_str("pub fn x() {}\n")?; + Ok(temp) +} + +#[test] +fn workspace_targets_multiply_matrix_rows() -> eyre::Result<()> { + let temp = workspace( + r#" + [workspace] + members = ["member"] + resolver = "2" + + [workspace.metadata.cargo-fc] + targets = ["ws-a", "ws-b"] + "#, + )?; + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + assert_eq!( + targets_in(&rows), + HashSet::from(["ws-a".to_string(), "ws-b".to_string()]) + ); + assert!( + rows.iter() + .all(|r| r.get("name").and_then(serde_json::Value::as_str) == Some("member")) + ); + Ok(()) +} + +#[test] +fn package_targets_override_workspace_targets() -> eyre::Result<()> { + let temp = TempDir::new()?; + temp.child("Cargo.toml").write_str( + r#" + [workspace] + members = ["member"] + resolver = "2" + + [workspace.metadata.cargo-fc] + targets = ["ws-a", "ws-b"] + "#, + )?; + temp.child("member/Cargo.toml").write_str( + r#" + [package] + name = "member" + version = "0.1.0" + edition = "2021" + [package.metadata.cargo-fc] + targets = ["pkg-only"] + "#, + )?; + temp.child("member/src/lib.rs") + .write_str("pub fn x() {}\n")?; + + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::default(); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + assert_eq!(targets_in(&rows), HashSet::from(["pkg-only".to_string()])); + Ok(()) +} + +#[test] +fn workspace_target_override_excludes_package_for_matching_target() -> eyre::Result<()> { + let temp = TempDir::new()?; + temp.child("Cargo.toml").write_str( + r#" + [workspace] + members = ["keep", "drop"] + resolver = "2" + + [workspace.metadata.cargo-fc] + targets = ["t-linux", "t-wasm"] + + [workspace.metadata.cargo-fc.target.'cfg(target_arch = "wasm32")'] + exclude_packages = { add = ["drop"] } + "#, + )?; + for name in ["keep", "drop"] { + temp.child(format!("{name}/Cargo.toml")) + .write_str(&format!( + r#" + [package] + name = "{name}" + version = "0.1.0" + edition = "2021" + "# + ))?; + temp.child(format!("{name}/src/lib.rs")) + .write_str("pub fn x() {}\n")?; + } + + let meta = metadata(&temp)?; + let env = HostEnv("host-triple"); + let mut eval = PairEval::matching("t-wasm", "cfg(target_arch = \"wasm32\")"); + let rows = matrix_rows(&meta, None, true, &env, &mut eval)?; + + let set = row_set(&rows); + // `drop` participates for linux but is excluded for wasm. + assert!(set.contains(&("drop".to_string(), "t-linux".to_string(), String::new()))); + assert!(!set.contains(&("drop".to_string(), "t-wasm".to_string(), String::new()))); + // `keep` participates for both. + assert!(set.contains(&("keep".to_string(), "t-linux".to_string(), String::new()))); + assert!(set.contains(&("keep".to_string(), "t-wasm".to_string(), String::new()))); + Ok(()) +} + +#[test] +fn unavailable_target_with_override_fails_clearly() -> eyre::Result<()> { + // An invalid triple combined with a target override forces real cfg + // evaluation (rustc), which fails with a clear, triple-named error. + let temp = single_crate( + r#" + [package] + name = "solo" + version = "0.1.0" + edition = "2021" + [features] + a = [] + [package.metadata.cargo-fc] + targets = ["definitely-not-a-real-triple"] + [package.metadata.cargo-fc.target.'cfg(target_os = "linux")'] + exclude_features = { add = ["a"] } + "#, + )?; + let meta = metadata(&temp)?; + let ws_config = meta.workspace_config()?; + let packages = meta.candidate_packages_for_fc()?; + let configs: Vec = packages + .iter() + .map(|p| p.config()) + .collect::>>()?; + let selected: Vec> = packages + .iter() + .zip(&configs) + .map(|(package, config)| SelectedPackage { package, config }) + .collect(); + let base_exclude = meta.base_workspace_exclude_packages()?; + let env = HostEnv("host-triple"); + let mut eval = cargo_feature_combinations::cfg_eval::RustcCfgEvaluator::default(); + + let target_plans = build_target_plans( + &selected, + &ws_config, + &base_exclude, + None, + true, + &env, + &mut eval, + )?; + let err = build_execution_plans(&target_plans, &Options::default(), false, &mut eval) + .err() + .ok_or_eyre("expected unavailable target to fail")?; + assert!( + err.to_string().contains("definitely-not-a-real-triple") + || format!("{err:?}").contains("definitely-not-a-real-triple"), + "error should name the offending triple: {err:?}" + ); + Ok(()) +} From 42b91afd6b6736110d2222d0933e2cab0d32619e Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:23:43 +0200 Subject: [PATCH 03/13] lint: deny expect attribute without explicit reason --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 1d98702..e58fa0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,7 @@ todo = "deny" indexing_slicing = "deny" # Every `#[allow(...)]` must carry a `reason = "..."` justifying the escape hatch. allow_attributes_without_reason = "deny" +expect_attributes_without_reason = "deny" struct_excessive_bools = "allow" From 859c3b9083b755dca3b15051a8024213b7b02b29 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:24:00 +0200 Subject: [PATCH 04/13] docs: simplify readme --- README.md | 57 ++----------------------------------------------------- 1 file changed, 2 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 2896937..ad4a4ce 100644 --- a/README.md +++ b/README.md @@ -281,9 +281,6 @@ into a full matrix of selected packages × effective targets × feature combinations ``` -so that a plain local `cargo fc check` exercises exactly the target cfg views -that CI exercises. - Declare workspace-wide targets in the workspace `Cargo.toml`: ```toml @@ -299,7 +296,7 @@ Individual packages can override the workspace list, or opt out of it: ```toml [package.metadata.cargo-fc] -# Run only this package on wasm (overrides the workspace list, does not merge). +# Run this package only on wasm (overrides the workspace list, does not merge). targets = ["wasm32-unknown-unknown"] # Or opt out of configured targets entirely and use the single effective target: @@ -342,8 +339,7 @@ flag. Built-in subcommands cargo-fc recognizes — `check`, `clippy`, `build`, automatically. Unknown aliases and custom subcommands do **not** receive configured targets -unless you opt them in. cargo-fc will not guess that `cargo lint` means -`cargo clippy`; instead, declare it: +unless you declare it: ```toml [workspace.metadata.cargo-fc.subcommands.lint] @@ -381,55 +377,6 @@ Workspace target overrides may patch `exclude_packages` only, and they apply to every concrete effective target — including single-target runs selected by `--target`, `CARGO_BUILD_TARGET`, or the host. -#### Matrix output - -Every `cargo fc matrix` row now includes a `target` field: - -```json -{ "name": "my-crate", "target": "x86_64-pc-windows-msvc", "features": "serde,cli" } -``` - -`target` is a reserved built-in key: if your `matrix` metadata already defines -`target`, the built-in value wins and cargo-fc warns. (This is an additive -schema change — runs with no configured targets still emit `target` = the host -triple on every row.) - -#### Execution modes - -cargo-fc is single-threaded and never spawns concurrent Cargo processes; it -relies on Cargo's own scheduler to use your cores. There are two modes: - -- **serial per-target (default)** — one Cargo invocation per - `(package, target, combination)`. Output stays live and every PASS/FAIL, - diagnostic, and dedupe note is attributed to exactly one target. -- **`--aggregate-targets`** — one Cargo invocation per `(package, combination)` - that passes every target sharing that combination as repeated `--target` - flags, letting Cargo overlap their build graphs. Faster on many-core machines, - but results are reported per target *group* (`targets = [a, b, …]`) rather - than per target. - -A worker pool of concurrent Cargo processes was measured and rejected: with a -shared `target/` directory, Cargo serializes on its build-directory lock, so -concurrent `--target` builds give no speedup; per-target `CARGO_TARGET_DIR` -workers do parallelize but recompile shared host artifacts per target (a small -win on many cores, ~28% slower on 2 cores, plus doubled disk). A single -multi-target invocation (aggregate mode) is the only approach that is faster on -many cores and never slower on small CI runners. - -`--aggregate-targets` falls back to serial per-target when: - -- the subcommand is `run` (Cargo rejects multiple `--target` for `run`), -- pruned summaries are enabled (`--show-pruned` or `show_pruned` in config) — - pruning is target-specific, -- only one target is effectively planned, - -and it has no effect on `cargo fc matrix` (rows are always per target). In each -case cargo-fc prints a short note. Aggregate warning/error counts are for the -whole target group and may differ from serial per-target counts; pair -`--aggregate-targets` with `--dedupe` for the cleanest diagnostics output. - ---- - ### Target-specific configuration You can override configuration for specific targets using Cargo-style `cfg(...)` expressions. From a1e74ef6e14f751f8cdfc4127c8cd41a81c3456c Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:33:32 +0200 Subject: [PATCH 05/13] feat: add --no-targets flag to ignore configured target lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--no-targets` temporarily disables running for all configured targets and falls back to Cargo's default single effective target (`--target`, then `CARGO_BUILD_TARGET`, then host) — an alternative to passing an explicit `--target `. It denies the target capability up front (without the "no targets capability" warning) so it also collapses `cargo fc matrix` to a single target; an explicit `--target` still wins. Document it in the README alongside the existing `--target` guidance. --- README.md | 11 ++++++++--- src/cli.rs | 12 ++++++++++++ src/lib.rs | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ad4a4ce..6de0689 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,8 @@ OPTIONS: --aggregate-targets Batch a combination's configured targets into one Cargo invocation (faster on many cores; group-level attribution). See "Configured targets" below. + --no-targets Ignore configured target lists and use Cargo's + default single target. See "Configured targets". ``` ### Configuration @@ -329,7 +331,8 @@ When the selected command supports targets, each package's targets are resolved > not be silently collapsed by a developer's ambient environment. This differs > from Cargo's own `[build].target` precedence. To run a single target for one > invocation, pass an explicit `--target `, which overrides all -> configured lists. +> configured lists, or pass `--no-targets` to ignore the configured lists and +> fall back to Cargo's default single target. #### Which commands receive configured targets @@ -354,8 +357,10 @@ cargo-fc warns once and falls back to the single effective target. > by `check`/`clippy` (which only need the target's `rustc`), but it also > applies to `build` (needs a linker), and to `test`/`run` (which execute and > therefore usually fail for foreign targets). Narrow a single run with an -> explicit `--target ` when needed. Missing targets surface Cargo's own -> `rustup target add ` hint; cargo-fc does not install targets for you. +> explicit `--target ` when needed, or pass `--no-targets` to ignore +> the configured lists entirely and use Cargo's default single target. Missing +> targets surface Cargo's own `rustup target add ` hint; cargo-fc does +> not install targets for you. #### Per-target workspace package selection diff --git a/src/cli.rs b/src/cli.rs index 49e77b5..9911508 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -82,6 +82,13 @@ pub struct Options { /// `(package, combo)` carries every target sharing that combo as repeated /// `--target` flags. The default (flag absent) is serial per-target. pub aggregate_targets: bool, + /// Whether to ignore configured target lists for this invocation. + /// + /// Set by `--no-targets`. Temporarily disables running for all configured + /// targets and falls back to Cargo's default single effective target + /// (`--target`, then `CARGO_BUILD_TARGET`, then host). An alternative to + /// passing an explicit `--target `. + pub no_targets: bool, } /// Helper trait to provide simple argument parsing over `Vec`. @@ -384,6 +391,10 @@ OPTIONS: instead of one invocation per target. Faster on many cores; reports results per target group. Falls back to serial for `run` and pruned summaries. + --no-targets Ignore configured target lists for this invocation + and use Cargo's default single target (--target, + then CARGO_BUILD_TARGET, then host). An alternative + to passing an explicit --target . Feature sets can be configured in your Cargo.toml configuration. The following metadata key aliases are all supported: @@ -622,6 +633,7 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec)> { drain_flag("--no-prune-implied", &mut options.no_prune_implied); drain_flag("--show-pruned", &mut options.show_pruned); drain_flag("--aggregate-targets", &mut options.aggregate_targets); + drain_flag("--no-targets", &mut options.no_targets); // --dedupe implies --diagnostics-only for flag in ["--dedupe", "--dedup"] { diff --git a/src/lib.rs b/src/lib.rs index 9cdd7a4..69038a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -322,6 +322,12 @@ fn resolve_capability_and_warn( ws_config: &config::WorkspaceConfig, selected: &[target_plan::SelectedPackage<'_>], ) -> bool { + // `--no-targets` deliberately ignores configured target lists and falls back + // to Cargo's default single target, so deny without warning. + if options.no_targets { + return false; + } + let is_matrix = matches!(options.command, Some(Command::FeatureMatrix { .. })); let (capability_allowed, policy) = if is_matrix { (true, None) @@ -468,6 +474,37 @@ mod test { ); } + #[test] + fn no_targets_flag_denies_capability() { + let options = Options { + no_targets: true, + ..Options::default() + }; + let ws = config::WorkspaceConfig::default(); + // Even a target-capable built-in command is denied configured targets + // when `--no-targets` is set. + assert!(!resolve_capability_and_warn(&options, &["check"], &ws, &[])); + } + + #[test] + fn builtin_command_allows_capability_without_no_targets() { + let options = Options::default(); + let ws = config::WorkspaceConfig::default(); + assert!(resolve_capability_and_warn(&options, &["check"], &ws, &[])); + } + + #[test] + fn no_targets_flag_denies_capability_for_matrix() { + let options = Options { + no_targets: true, + command: Some(Command::FeatureMatrix { pretty: false }), + ..Options::default() + }; + let ws = config::WorkspaceConfig::default(); + let empty: [&str; 0] = []; + assert!(!resolve_capability_and_warn(&options, &empty, &ws, &[])); + } + #[test] fn find_metadata_value_returns_none_for_empty_object() { let meta = json!({}); From 3511eb6c0ad3690e85a156ed7d510a4656b91aef Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:35:04 +0200 Subject: [PATCH 06/13] fix: drop unrecognized expect_attributes_without_reason clippy lint --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e58fa0e..1d98702 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,7 +67,6 @@ todo = "deny" indexing_slicing = "deny" # Every `#[allow(...)]` must carry a `reason = "..."` justifying the escape hatch. allow_attributes_without_reason = "deny" -expect_attributes_without_reason = "deny" struct_excessive_bools = "allow" From 9a93225cf34ae38cbc77b1b32b890ed4ad65f27c Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:35:04 +0200 Subject: [PATCH 07/13] docs: note matrix custom data should live under a data subkey --- TODO.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index 0c36352..314a968 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,5 @@ #### TODO -- allow adding custom data to matrix output +- allow adding custom data to matrix output but surface under data subkey! - embed the help output using embedme. - add a github actions workflow file example. From f28731a4d05d935d1b436e9af448f9073fb694b7 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 22:35:25 +0200 Subject: [PATCH 08/13] update todo --- TODO.md | 1 - 1 file changed, 1 deletion(-) diff --git a/TODO.md b/TODO.md index 314a968..b5dbc84 100644 --- a/TODO.md +++ b/TODO.md @@ -2,4 +2,3 @@ - allow adding custom data to matrix output but surface under data subkey! - embed the help output using embedme. -- add a github actions workflow file example. From dbfe1eca880e1f4961ddfae099a2328160c250e5 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 23:26:48 +0200 Subject: [PATCH 09/13] refactor: simplify target-capability resolution and metadata-section helpers - Collapse the command target-capability "policy" ceremony to a single `command_allows_configured_targets(...) -> bool`, dropping the `ResolvedCommandTargetPolicy` struct, the `CommandCapabilitySource` and `CapabilityDecision` enums, and `is_builtin_target_command`'s `Option`. - Drop dead code: `TargetSource::is_configured`, the two `distinct_targets` helpers, and `RustcTargetEnvironment`'s generic + `with_env` (now a unit struct reading the environment directly). - Have `pkg_metadata_section` / `ws_metadata_section` / `DEFAULT_PKG_METADATA_SECTION` return the dotted path; callers wrap `[...]` and append sub-keys (e.g. `.subcommands.`). Capability warnings/hints reuse `ws_metadata_section` and echo the user's actual metadata alias instead of hardcoding `cargo-fc`. --- src/cli.rs | 157 +++++++++++++++------------------------------ src/lib.rs | 94 +++++++++++++++++---------- src/package.rs | 6 +- src/runner.rs | 10 +-- src/target.rs | 29 ++------- src/target_plan.rs | 8 --- src/workspace.rs | 6 +- 7 files changed, 122 insertions(+), 188 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 9911508..5a4fde2 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -242,116 +242,49 @@ pub(crate) fn cargo_subcommand_token(args: &[impl AsRef]) -> Option None } -/// Built-in cargo subcommand tokens cargo-fc knows accept Cargo's `--target`. +/// Whether `token` is a built-in cargo subcommand cargo-fc knows accepts +/// Cargo's `--target` flag. /// /// This is the initial target-capability registry. It tracks the command /// tokens cargo-fc already recognizes today (`build`, `check`, `clippy`, /// `test`, `doc`, `run`) plus their short aliases. `matrix` is handled /// separately because it is not a forwarded cargo command. -fn builtin_target_capability(token: &str) -> Option { - match token { - "build" | "b" | "check" | "c" | "clippy" | "test" | "t" | "doc" | "d" | "run" | "r" => { - Some(true) - } - _ => None, - } -} - -/// Where a command's target capability was decided. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CommandCapabilitySource { - /// A known built-in cargo subcommand. - BuiltIn, - /// An alias/custom command opted in via workspace config. - WorkspaceConfig, - /// An unknown command with no built-in entry and no workspace opt-in. - Unknown, +fn is_builtin_target_command(token: &str) -> bool { + matches!( + token, + "build" | "b" | "check" | "c" | "clippy" | "test" | "t" | "doc" | "d" | "run" | "r" + ) } -/// Whether configured targets / `--target` injection apply to a command. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CapabilityDecision { - /// cargo-fc may expand configured target lists and inject `--target`. - Allowed, - /// Configured target lists are ignored for this command. - Denied, -} - -impl CapabilityDecision { - /// Whether the decision allows configured targets. - #[must_use] - pub fn is_allowed(self) -> bool { - matches!(self, Self::Allowed) - } -} - -/// Resolved target-capability policy for the selected command. -/// -/// The runner consumes one resolved policy object instead of repeatedly asking -/// whether a subcommand is known. -#[derive(Debug, Clone)] -pub struct ResolvedCommandTargetPolicy { - /// The detected command token (e.g. `clippy`, `lint`), or `matrix`. - pub command_name: String, - /// Where the capability decision came from. - pub source: CommandCapabilitySource, - /// Whether configured targets apply to this command. - pub targets: CapabilityDecision, -} - -/// Resolve the target-capability policy for a forwarded cargo command token. +/// Whether the selected command may expand configured target lists and have +/// `--target ` injected. /// /// Built-in commands get their capability from code and ignore workspace -/// config. If a workspace `subcommands.` entry shadows a built-in -/// command, it is ignored with a warning. Unknown tokens are denied unless the -/// workspace opts them in with `targets = true`. +/// config; a workspace `subcommands.` entry that shadows a built-in is +/// ignored with a warning. Unknown tokens are denied unless the workspace opts +/// them in with `targets = true`. `metadata_key` is the alias the workspace +/// metadata was found under, so the shadow warning echoes the user's own alias. #[must_use] -pub fn resolve_command_target_policy( +pub fn command_allows_configured_targets( token: Option<&str>, subcommands: &std::collections::BTreeMap, -) -> ResolvedCommandTargetPolicy { + metadata_key: &str, +) -> bool { let Some(token) = token else { - return ResolvedCommandTargetPolicy { - command_name: String::new(), - source: CommandCapabilitySource::Unknown, - targets: CapabilityDecision::Denied, - }; + return false; }; - if let Some(allowed) = builtin_target_capability(token) { + if is_builtin_target_command(token) { if subcommands.contains_key(token) { crate::print_warning!( - "ignoring [workspace.metadata.cargo-fc.subcommands.{token}]: `{token}` is a built-in cargo subcommand whose target capability is provided by cargo-fc" + "ignoring [{}.subcommands.{token}]: `{token}` is a built-in cargo subcommand whose target capability is provided by cargo-fc", + crate::ws_metadata_section(metadata_key), ); } - return ResolvedCommandTargetPolicy { - command_name: token.to_string(), - source: CommandCapabilitySource::BuiltIn, - targets: if allowed { - CapabilityDecision::Allowed - } else { - CapabilityDecision::Denied - }, - }; - } - - if let Some(capability) = subcommands.get(token) { - return ResolvedCommandTargetPolicy { - command_name: token.to_string(), - source: CommandCapabilitySource::WorkspaceConfig, - targets: if capability.targets { - CapabilityDecision::Allowed - } else { - CapabilityDecision::Denied - }, - }; - } - - ResolvedCommandTargetPolicy { - command_name: token.to_string(), - source: CommandCapabilitySource::Unknown, - targets: CapabilityDecision::Denied, + return true; } + + subcommands.get(token).is_some_and(|c| c.targets) } static VALID_BOOLS: [&str; 4] = ["yes", "true", "y", "t"]; @@ -668,8 +601,8 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec)> { #[cfg(test)] mod test { use super::{ - CapabilityDecision, CargoSubcommand, CommandCapabilitySource, cargo_subcommand, - cargo_subcommand_token, resolve_command_target_policy, + CargoSubcommand, cargo_subcommand, cargo_subcommand_token, + command_allows_configured_targets, }; use crate::config::CommandTargetCapability; use similar_asserts::assert_eq as sim_assert_eq; @@ -776,26 +709,32 @@ mod test { #[test] fn builtin_clippy_has_target_capability() { let subcommands = BTreeMap::new(); - let policy = resolve_command_target_policy(Some("clippy"), &subcommands); - sim_assert_eq!(policy.source, CommandCapabilitySource::BuiltIn); - sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + assert!(command_allows_configured_targets( + Some("clippy"), + &subcommands, + "cargo-fc" + )); } #[test] fn unknown_lint_lacks_target_capability_unless_configured() { let empty = BTreeMap::new(); - let policy = resolve_command_target_policy(Some("lint"), &empty); - sim_assert_eq!(policy.source, CommandCapabilitySource::Unknown); - sim_assert_eq!(policy.targets, CapabilityDecision::Denied); + assert!(!command_allows_configured_targets( + Some("lint"), + &empty, + "cargo-fc" + )); let mut configured = BTreeMap::new(); configured.insert( "lint".to_string(), CommandTargetCapability { targets: true }, ); - let policy = resolve_command_target_policy(Some("lint"), &configured); - sim_assert_eq!(policy.source, CommandCapabilitySource::WorkspaceConfig); - sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + assert!(command_allows_configured_targets( + Some("lint"), + &configured, + "cargo-fc" + )); } #[test] @@ -805,9 +744,11 @@ mod test { "lint".to_string(), CommandTargetCapability { targets: false }, ); - let policy = resolve_command_target_policy(Some("lint"), &configured); - sim_assert_eq!(policy.source, CommandCapabilitySource::WorkspaceConfig); - sim_assert_eq!(policy.targets, CapabilityDecision::Denied); + assert!(!command_allows_configured_targets( + Some("lint"), + &configured, + "cargo-fc" + )); } #[test] @@ -818,8 +759,10 @@ mod test { CommandTargetCapability { targets: false }, ); // A workspace entry shadowing a built-in must not flip its capability. - let policy = resolve_command_target_policy(Some("check"), &configured); - sim_assert_eq!(policy.source, CommandCapabilitySource::BuiltIn); - sim_assert_eq!(policy.targets, CapabilityDecision::Allowed); + assert!(command_allows_configured_targets( + Some("check"), + &configured, + "cargo-fc" + )); } } diff --git a/src/lib.rs b/src/lib.rs index 69038a9..8fde08a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -119,9 +119,10 @@ pub(crate) const METADATA_KEYS: &[&str] = &[ /// usage is detected. pub(crate) const DEFAULT_METADATA_KEY: &str = default_metadata_key!(); -/// Default TOML section header for per-package configuration. +/// Default dotted `package.metadata.` path for per-package configuration +/// (no brackets; callers wrap it in `[...]`). pub(crate) const DEFAULT_PKG_METADATA_SECTION: &str = - concat!("[package.metadata.", default_metadata_key!(), "]"); + concat!("package.metadata.", default_metadata_key!()); /// Look up configuration from any recognized metadata key alias. /// @@ -138,14 +139,20 @@ pub(crate) fn find_metadata_value( None } -/// Format a `[package.metadata.]` TOML section header. +/// Format the dotted `package.metadata.` path (no brackets). +/// +/// Callers wrap it in `[...]` and may append a sub-key, e.g. +/// `[{pkg_metadata_section(key)}.target.'cfg(...)']`. pub(crate) fn pkg_metadata_section(key: &str) -> String { - format!("[package.metadata.{key}]") + format!("package.metadata.{key}") } -/// Format a `[workspace.metadata.]` TOML section header. +/// Format the dotted `workspace.metadata.` path (no brackets). +/// +/// Callers wrap it in `[...]` and may append a sub-key, e.g. +/// `[{ws_metadata_section(key)}.subcommands.]`. pub(crate) fn ws_metadata_section(key: &str) -> String { - format!("[workspace.metadata.{key}]") + format!("workspace.metadata.{key}") } /// Run the cargo subcommand for all relevant feature combinations. @@ -203,10 +210,13 @@ pub fn run(bin_name: &str) -> eyre::Result<()> { // Parse an explicit `--target` only before `--`. let cli_target = target::parse_cli_target(&cargo_args_owned); + // Echo the user's own metadata alias in capability hints/warnings. + let ws_key = find_metadata_value(&metadata.workspace_metadata) + .map_or(DEFAULT_METADATA_KEY, |(_, key)| key); let capability_allowed = - resolve_capability_and_warn(&options, &cargo_args, &ws_config, &selected); + resolve_capability_and_warn(&options, &cargo_args, &ws_config, ws_key, &selected); - let env = RustcTargetEnvironment::default(); + let env = RustcTargetEnvironment; let mut evaluator = RustcCfgEvaluator::default(); let base_exclude = metadata.base_workspace_exclude_packages()?; @@ -320,6 +330,7 @@ fn resolve_capability_and_warn( options: &Options, cargo_args: &[&str], ws_config: &config::WorkspaceConfig, + ws_key: &str, selected: &[target_plan::SelectedPackage<'_>], ) -> bool { // `--no-targets` deliberately ignores configured target lists and falls back @@ -328,35 +339,34 @@ fn resolve_capability_and_warn( return false; } - let is_matrix = matches!(options.command, Some(Command::FeatureMatrix { .. })); - let (capability_allowed, policy) = if is_matrix { - (true, None) - } else { - let token = cli::cargo_subcommand_token(cargo_args); - let policy = cli::resolve_command_target_policy(token.as_deref(), &ws_config.subcommands); - (policy.targets.is_allowed(), Some(policy)) - }; + // `matrix` is not a forwarded cargo command: it always uses configured + // target planning. + if matches!(options.command, Some(Command::FeatureMatrix { .. })) { + return true; + } + let token = cli::cargo_subcommand_token(cargo_args); + if cli::command_allows_configured_targets(token.as_deref(), &ws_config.subcommands, ws_key) { + return true; + } + + // Capability denied: warn (once) only when the user actually configured + // targets that we are now skipping. let has_raw_configured_targets = !ws_config.targets.is_empty() || selected .iter() .any(|s| s.config.targets.as_ref().is_some_and(|t| !t.is_empty())); - if !capability_allowed - && has_raw_configured_targets - && let Some(policy) = &policy - && !policy.command_name.is_empty() - { + if has_raw_configured_targets && let Some(token) = token.as_deref().filter(|t| !t.is_empty()) { print_warning!( - "not passing configured targets to cargo command `{}` because it has no targets capability", - policy.command_name, + "not passing configured targets to cargo command `{token}` because it has no targets capability" ); eprintln!( - "hint: add [workspace.metadata.cargo-fc.subcommands.{}] targets = true if this command accepts --target", - policy.command_name, + "hint: add [{}.subcommands.{token}] targets = true if this command accepts --target", + ws_metadata_section(ws_key), ); } - capability_allowed + false } /// Resolve the effective target execution mode, emitting a note when an @@ -483,14 +493,26 @@ mod test { let ws = config::WorkspaceConfig::default(); // Even a target-capable built-in command is denied configured targets // when `--no-targets` is set. - assert!(!resolve_capability_and_warn(&options, &["check"], &ws, &[])); + assert!(!resolve_capability_and_warn( + &options, + &["check"], + &ws, + DEFAULT_METADATA_KEY, + &[] + )); } #[test] fn builtin_command_allows_capability_without_no_targets() { let options = Options::default(); let ws = config::WorkspaceConfig::default(); - assert!(resolve_capability_and_warn(&options, &["check"], &ws, &[])); + assert!(resolve_capability_and_warn( + &options, + &["check"], + &ws, + DEFAULT_METADATA_KEY, + &[] + )); } #[test] @@ -502,7 +524,13 @@ mod test { }; let ws = config::WorkspaceConfig::default(); let empty: [&str; 0] = []; - assert!(!resolve_capability_and_warn(&options, &empty, &ws, &[])); + assert!(!resolve_capability_and_warn( + &options, + &empty, + &ws, + DEFAULT_METADATA_KEY, + &[] + )); } #[test] @@ -556,16 +584,16 @@ mod test { fn pkg_metadata_section_formats_correctly() { assert_eq!( pkg_metadata_section("cargo-fc"), - "[package.metadata.cargo-fc]" + "package.metadata.cargo-fc" ); - assert_eq!(pkg_metadata_section("fc"), "[package.metadata.fc]"); + assert_eq!(pkg_metadata_section("fc"), "package.metadata.fc"); } #[test] fn ws_metadata_section_formats_correctly() { assert_eq!( ws_metadata_section("cargo-fc"), - "[workspace.metadata.cargo-fc]" + "workspace.metadata.cargo-fc" ); } @@ -576,6 +604,6 @@ mod test { #[test] fn default_pkg_metadata_section_uses_default_key() { - assert_eq!(DEFAULT_PKG_METADATA_SECTION, "[package.metadata.cargo-fc]"); + assert_eq!(DEFAULT_PKG_METADATA_SECTION, "package.metadata.cargo-fc"); } } diff --git a/src/package.rs b/src/package.rs index a00f9cc..29d640e 100644 --- a/src/package.rs +++ b/src/package.rs @@ -92,21 +92,21 @@ impl Package for cargo_metadata::Package { if !config.deprecated.skip_feature_sets.is_empty() { print_warning!( - "{section}.skip_feature_sets in package `{}` is deprecated; use exclude_feature_sets instead", + "[{section}].skip_feature_sets in package `{}` is deprecated; use exclude_feature_sets instead", self.name, ); } if !config.deprecated.denylist.is_empty() { print_warning!( - "{section}.denylist in package `{}` is deprecated; use exclude_features instead", + "[{section}].denylist in package `{}` is deprecated; use exclude_features instead", self.name, ); } if !config.deprecated.exact_combinations.is_empty() { print_warning!( - "{section}.exact_combinations in package `{}` is deprecated; use include_feature_sets instead", + "[{section}].exact_combinations in package `{}` is deprecated; use include_feature_sets instead", self.name, ); } diff --git a/src/runner.rs b/src/runner.rs index 65b0e11..ed3cecd 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -244,7 +244,7 @@ pub(crate) fn print_feature_combination_error(err: &FeatureCombinationError) { let _ = stderr.reset(); let _ = writeln!( &mut stderr, - " Consider restricting the matrix using {DEFAULT_PKG_METADATA_SECTION}.only_features", + " Consider restricting the matrix using [{DEFAULT_PKG_METADATA_SECTION}].only_features", ); let _ = writeln!( &mut stderr, @@ -635,14 +635,6 @@ pub struct ExecutionPlanSet<'a> { pub show_target: bool, } -impl ExecutionPlanSet<'_> { - /// The distinct target triples planned, in plan order. - #[must_use] - pub fn distinct_targets(&self) -> Vec { - self.plans.iter().map(|p| p.target.clone()).collect() - } -} - /// Build owned execution plans from target plans. /// /// Resolves each package assignment's target-specific config from the cached diff --git a/src/target.rs b/src/target.rs index 13138a5..5e8a5df 100644 --- a/src/target.rs +++ b/src/target.rs @@ -44,12 +44,6 @@ impl TargetSource { pub fn should_inject_target_arg(self) -> bool { matches!(self, Self::PackageConfig | Self::WorkspaceConfig) } - - /// Whether this assignment came from configured target metadata. - #[must_use] - pub fn is_configured(self) -> bool { - matches!(self, Self::PackageConfig | Self::WorkspaceConfig) - } } /// A concrete target together with where it came from. @@ -78,27 +72,12 @@ pub trait TargetEnvironment { /// Production [`TargetEnvironment`] backed by the process environment and /// `rustc -vV`. -#[derive(Debug, Clone)] -pub struct RustcTargetEnvironment { - env: E, -} - -impl Default for RustcTargetEnvironment { - fn default() -> Self { - Self { env: ProcessEnv } - } -} - -impl RustcTargetEnvironment { - /// Create an environment adapter over a custom [`Env`]. - pub fn with_env(env: E) -> Self { - Self { env } - } -} +#[derive(Debug, Default, Clone, Copy)] +pub struct RustcTargetEnvironment; -impl TargetEnvironment for RustcTargetEnvironment { +impl TargetEnvironment for RustcTargetEnvironment { fn cargo_build_target(&self) -> Option { - let triple = self.env.var("CARGO_BUILD_TARGET")?; + let triple = std::env::var("CARGO_BUILD_TARGET").ok()?; let triple = triple.trim(); if triple.is_empty() { None diff --git a/src/target_plan.rs b/src/target_plan.rs index f013579..fd58169 100644 --- a/src/target_plan.rs +++ b/src/target_plan.rs @@ -63,14 +63,6 @@ pub struct TargetPlans<'a> { pub contains_configured_assignments: bool, } -impl TargetPlans<'_> { - /// The distinct target triples planned, in plan order. - #[must_use] - pub fn distinct_targets(&self) -> Vec { - self.plans.iter().map(|p| p.target.clone()).collect() - } -} - /// Trim, reject empty, and deduplicate a configured target list, preserving /// first-occurrence order. fn normalize_targets(raw: &[String]) -> eyre::Result> { diff --git a/src/workspace.rs b/src/workspace.rs index 7a01b65..e17d617 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -112,7 +112,7 @@ fn warn_workspace_metadata_misuse(metadata: &cargo_metadata::Metadata) { && !config.exclude_packages.is_empty() { print_warning!( - "{}.exclude_packages in the workspace root package is deprecated; use {}.exclude_packages instead", + "[{}].exclude_packages in the workspace root package is deprecated; use [{}].exclude_packages instead", pkg_metadata_section(root_key), ws_metadata_section(root_key), ); @@ -130,7 +130,7 @@ fn warn_workspace_metadata_misuse(metadata: &cargo_metadata::Metadata) { && !config.exclude_packages.is_empty() { print_warning!( - "{}.exclude_packages in package `{}` has no effect; this field is only read from the workspace root Cargo.toml", + "[{}].exclude_packages in package `{}` has no effect; this field is only read from the workspace root Cargo.toml", pkg_metadata_section(key), package.name, ); @@ -147,7 +147,7 @@ fn warn_workspace_metadata_misuse(metadata: &cargo_metadata::Metadata) { for ws_key in WORKSPACE_ONLY_KEYS { if json_has_values(tool.get(*ws_key)) { print_warning!( - "{}.{} in package `{}` has no effect; workspace metadata is only read from the workspace root Cargo.toml", + "[{}].{} in package `{}` has no effect; workspace metadata is only read from the workspace root Cargo.toml", ws_metadata_section(key), ws_key, package.name, From 798e04706b52b35770b8334018041fd2c1ac96c7 Mon Sep 17 00:00:00 2001 From: romnn Date: Sun, 28 Jun 2026 23:54:49 +0200 Subject: [PATCH 10/13] feat: allow command target policy overrides --- README.md | 16 ++- plan/per-target-feature-combinations.md | 77 +++++++----- src/cli.rs | 151 +++++++++++++++--------- src/config.rs | 16 +-- src/lib.rs | 26 +++- 5 files changed, 191 insertions(+), 95 deletions(-) diff --git a/README.md b/README.md index 6de0689..70724a5 100644 --- a/README.md +++ b/README.md @@ -339,7 +339,7 @@ When the selected command supports targets, each package's targets are resolved Configured targets are applied only to commands that accept Cargo's `--target` flag. Built-in subcommands cargo-fc recognizes — `check`, `clippy`, `build`, `doc`, `test`, `run` (and `cargo fc matrix`) — get this capability -automatically. +automatically by default. Unknown aliases and custom subcommands do **not** receive configured targets unless you declare it: @@ -349,8 +349,18 @@ unless you declare it: targets = true ``` -If configured targets exist but the selected command lacks this capability, -cargo-fc warns once and falls back to the single effective target. +The same table can override built-in defaults. For example, lint every +configured target but keep `cargo fc build` on the single effective target: + +```toml +[workspace.metadata.cargo-fc.subcommands.build] +targets = false +``` + +For built-in short aliases, the long command's policy also applies unless the +short alias has its own entry. If configured targets exist but the selected +command lacks this capability by default, cargo-fc warns once and falls back to +the single effective target. An explicit `targets = false` opt-out is quiet. > [!WARNING] > The `targets` list is shared by all target-capable commands. It is motivated diff --git a/plan/per-target-feature-combinations.md b/plan/per-target-feature-combinations.md index 7b6cdf4..e3d9390 100644 --- a/plan/per-target-feature-combinations.md +++ b/plan/per-target-feature-combinations.md @@ -373,9 +373,10 @@ consumers to migrate immediately. ## Command Scope Configured target lists should apply only when the selected cargo subcommand has -the target capability. Built-in Cargo subcommands get this capability from a -small built-in registry. Unknown aliases get no target capability unless the -workspace config opts them in. +the target capability. Built-in Cargo subcommands default to this capability via +a small built-in registry. Unknown aliases default to no target capability +unless the workspace config opts them in. Workspace config may override either +default per command token. This avoids guessing that `cargo lint` means `cargo clippy`. `lint` could be any Cargo alias or custom command, so it must remain unknown unless the workspace @@ -397,7 +398,10 @@ Known built-in commands should have explicit built-in target allowlist entries. The allowlist is not only for Cargo's built-in binaries; it is for command tokens that cargo-fc knows how to reason about. For example, `clippy` must be in the built-in allowlist because cargo-fc knows `cargo clippy` accepts Cargo's -`--target` flag. +`--target` flag. The built-in entry is a default, not a hard lock: users may set +`subcommands..targets = false` to keep a built-in command on the single +effective target while still running other commands across the configured target +list. Encode the table below as the initial built-in registry. If implementation tests prove that a command does not actually accept `--target`, adjust the @@ -501,10 +505,13 @@ and before target planning: 1. Detect the raw cargo subcommand token using the same cargo-flag skipping rules currently used by `cargo_subcommand`. This needs a token-extracting helper because the current enum collapses all unknown commands to `Other`. -2. If the token maps to a known built-in cargo subcommand, use the built-in - target-capability registry. -3. Otherwise, look up `[workspace.metadata.cargo-fc.subcommands.]`. -4. If no configured entry exists, deny configured targets for that command. +2. Look up `[workspace.metadata.cargo-fc.subcommands.]`. +3. For built-in short aliases, if there is no exact alias entry, also look up + the long built-in command key. +4. If no configured entry exists and the token maps to a known built-in cargo + subcommand, use the built-in target-capability registry. +5. If no configured entry exists for an unknown command, deny configured + targets for that command. The `cargo fc matrix` command is not a forwarded cargo subcommand: it has target capability unconditionally and skips token detection entirely. The resolution @@ -535,11 +542,11 @@ The exact type names can differ. The important design rule is that the runner consumes one resolved policy object instead of repeatedly asking whether a subcommand is known. -Built-in commands should not require workspace config. If a workspace config -entry uses the same name as a built-in command, either reject it or warn that it -is ignored. Do not silently let a config table change built-in command behavior -in the first implementation; that would make standard cargo-fc behavior harder -to reason about. +Built-in commands should not require workspace config. Workspace config is still +allowed to override their default target policy, because "should cargo-fc apply +the configured target axis for this command?" is a repo policy decision. This +supports workflows such as linting every configured target while keeping +`cargo fc build` on the single effective target. The target warning must be driven from raw config state, not from the planned targets after capability filtering. Before planning, compute whether any @@ -663,8 +670,7 @@ those values into the base exclude set before target-specific patches. ### Command Target Capability Config -Add a workspace-level target-capability table for aliases and custom cargo -subcommands: +Add a workspace-level target-capability table for cargo subcommands: ```toml [workspace.metadata.cargo-fc.subcommands.lint] @@ -674,6 +680,17 @@ targets = true This means: when the detected cargo subcommand is `lint`, cargo-fc is allowed to expand configured target lists and inject `--target `. +The same table may override built-in defaults: + +```toml +[workspace.metadata.cargo-fc.subcommands.build] +targets = false +``` + +This means: `cargo fc build` uses the single effective target unless the user +passes an explicit Cargo `--target`, while other commands can still use the +workspace/package target lists. + This table must not change generated feature-selection behavior or diagnostics behavior in v1. @@ -687,36 +704,37 @@ pub struct CommandTargetCapability { } ``` -A plain `bool` (default `false`) is enough in v1. Only unknown aliases consult -this table and the default is deny, so "omitted" and "`targets = false`" are -indistinguishable in practice; `Option` would add a distinction with no -observable effect. If a future version grows more capabilities with non-deny -defaults, switch the relevant fields to `Option` then. +A plain `bool` (default `false`) is enough in v1. Unknown aliases default to +deny, while built-ins default according to the registry. A present +`targets = false` entry is an explicit opt-out and should suppress the unknown +command "how to opt in" warning for that token. For unknown aliases the default is therefore `targets = false` (denied). -Built-in commands get their capability from code and ignore this table. +Built-in commands get their default from code, and this table may override it. Keep this table in workspace metadata only. Command aliases are an invocation property, not a package property. Per-package command target capability would make a single `cargo fc lint` invocation ambiguous when selected packages disagree. -Built-in target capability should be provided by code. Workspace config is for -aliases/custom commands only. Do not require users to configure standard Cargo -subcommands such as `check`, `clippy`, or `build`, and do not let workspace -config silently override built-in command behavior in the first implementation. +Built-in target capability should be provided by code. Do not require users to +configure standard Cargo subcommands such as `check`, `clippy`, or `build`, but +do allow workspace config to override those defaults intentionally. The `targets` capability name describes cargo-fc behavior: cargo-fc may expand configured target lists and add `--target `. -When configured targets exist and the selected command lacks target capability, -cargo-fc should skip the target expansion and warn. For example: +When configured targets exist and the selected command lacks target capability +by default, cargo-fc should skip the target expansion and warn. For example: ```text warning: not passing --target to cargo alias `lint` because it has no configured targets capability hint: add [workspace.metadata.cargo-fc.subcommands.lint] targets = true if this alias accepts --target ``` +When the command is explicitly configured with `targets = false`, cargo-fc +should skip target expansion without warning. + ### Package Config Add package-level targets to `Config`: @@ -1586,8 +1604,9 @@ Closed decisions: - Target plans are deduplicated by target triple, while package-target assignments carry `TargetSource` for per-package injection decisions. - Workspace target overrides may patch only `exclude_packages`. -- Command target capability is workspace-level policy for aliases. Built-in - `clippy` is known; literal `lint` is unknown unless configured. +- Command target capability is workspace-level policy. Built-in `clippy` is + known by default; literal `lint` is unknown unless configured; any command + token may be explicitly overridden with `targets = true/false`. - Unknown aliases retain legacy best-effort feature selection, but configured targets require explicit target capability. - Existing `--diagnostics-only`/`--dedupe` eligibility is preserved in v1. diff --git a/src/cli.rs b/src/cli.rs index 5a4fde2..207f4c5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -242,49 +242,80 @@ pub(crate) fn cargo_subcommand_token(args: &[impl AsRef]) -> Option None } -/// Whether `token` is a built-in cargo subcommand cargo-fc knows accepts -/// Cargo's `--target` flag. +/// Canonical long command name for built-in cargo subcommands cargo-fc knows +/// accept Cargo's `--target` flag. /// /// This is the initial target-capability registry. It tracks the command /// tokens cargo-fc already recognizes today (`build`, `check`, `clippy`, /// `test`, `doc`, `run`) plus their short aliases. `matrix` is handled /// separately because it is not a forwarded cargo command. -fn is_builtin_target_command(token: &str) -> bool { - matches!( - token, - "build" | "b" | "check" | "c" | "clippy" | "test" | "t" | "doc" | "d" | "run" | "r" - ) +fn builtin_target_command_key(token: &str) -> Option<&'static str> { + match token { + "build" | "b" => Some("build"), + "check" | "c" => Some("check"), + "clippy" => Some("clippy"), + "test" | "t" => Some("test"), + "doc" | "d" => Some("doc"), + "run" | "r" => Some("run"), + _ => None, + } +} + +/// Resolved configured-target policy for a cargo command token. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct ConfiguredTargetPolicy { + /// Whether configured target lists apply to this command. + pub enabled: bool, + /// Whether the decision came from workspace config. + /// + /// This lets callers avoid warning when `targets = false` is an intentional + /// opt-out rather than the unknown-command default. + pub explicit: bool, } /// Whether the selected command may expand configured target lists and have /// `--target ` injected. /// -/// Built-in commands get their capability from code and ignore workspace -/// config; a workspace `subcommands.` entry that shadows a built-in is -/// ignored with a warning. Unknown tokens are denied unless the workspace opts -/// them in with `targets = true`. `metadata_key` is the alias the workspace -/// metadata was found under, so the shadow warning echoes the user's own alias. +/// Built-in commands default to `targets = true`; unknown aliases default to +/// `targets = false`. Workspace `subcommands..targets` overrides either +/// default. For built-in short aliases, an exact alias entry wins; otherwise the +/// long command's entry applies. #[must_use] -pub fn command_allows_configured_targets( +pub(crate) fn configured_target_policy( token: Option<&str>, subcommands: &std::collections::BTreeMap, - metadata_key: &str, -) -> bool { +) -> ConfiguredTargetPolicy { let Some(token) = token else { - return false; + return ConfiguredTargetPolicy { + enabled: false, + explicit: false, + }; }; - if is_builtin_target_command(token) { - if subcommands.contains_key(token) { - crate::print_warning!( - "ignoring [{}.subcommands.{token}]: `{token}` is a built-in cargo subcommand whose target capability is provided by cargo-fc", - crate::ws_metadata_section(metadata_key), - ); + if let Some(capability) = subcommands.get(token) { + return ConfiguredTargetPolicy { + enabled: capability.targets, + explicit: true, + }; + } + + if let Some(command_key) = builtin_target_command_key(token) { + if let Some(capability) = subcommands.get(command_key) { + return ConfiguredTargetPolicy { + enabled: capability.targets, + explicit: true, + }; } - return true; + return ConfiguredTargetPolicy { + enabled: true, + explicit: false, + }; } - subcommands.get(token).is_some_and(|c| c.targets) + ConfiguredTargetPolicy { + enabled: false, + explicit: false, + } } static VALID_BOOLS: [&str; 4] = ["yes", "true", "y", "t"]; @@ -601,8 +632,7 @@ pub fn parse_arguments(bin_name: &str) -> eyre::Result<(Options, Vec)> { #[cfg(test)] mod test { use super::{ - CargoSubcommand, cargo_subcommand, cargo_subcommand_token, - command_allows_configured_targets, + CargoSubcommand, cargo_subcommand, cargo_subcommand_token, configured_target_policy, }; use crate::config::CommandTargetCapability; use similar_asserts::assert_eq as sim_assert_eq; @@ -709,32 +739,26 @@ mod test { #[test] fn builtin_clippy_has_target_capability() { let subcommands = BTreeMap::new(); - assert!(command_allows_configured_targets( - Some("clippy"), - &subcommands, - "cargo-fc" - )); + let policy = configured_target_policy(Some("clippy"), &subcommands); + assert!(policy.enabled); + assert!(!policy.explicit); } #[test] fn unknown_lint_lacks_target_capability_unless_configured() { let empty = BTreeMap::new(); - assert!(!command_allows_configured_targets( - Some("lint"), - &empty, - "cargo-fc" - )); + let policy = configured_target_policy(Some("lint"), &empty); + assert!(!policy.enabled); + assert!(!policy.explicit); let mut configured = BTreeMap::new(); configured.insert( "lint".to_string(), CommandTargetCapability { targets: true }, ); - assert!(command_allows_configured_targets( - Some("lint"), - &configured, - "cargo-fc" - )); + let policy = configured_target_policy(Some("lint"), &configured); + assert!(policy.enabled); + assert!(policy.explicit); } #[test] @@ -744,25 +768,46 @@ mod test { "lint".to_string(), CommandTargetCapability { targets: false }, ); - assert!(!command_allows_configured_targets( - Some("lint"), - &configured, - "cargo-fc" - )); + let policy = configured_target_policy(Some("lint"), &configured); + assert!(!policy.enabled); + assert!(policy.explicit); } #[test] - fn builtin_capability_ignores_workspace_shadowing_entry() { + fn configured_builtin_with_targets_false_is_denied() { let mut configured = BTreeMap::new(); configured.insert( "check".to_string(), CommandTargetCapability { targets: false }, ); - // A workspace entry shadowing a built-in must not flip its capability. - assert!(command_allows_configured_targets( - Some("check"), - &configured, - "cargo-fc" - )); + let policy = configured_target_policy(Some("check"), &configured); + assert!(!policy.enabled); + assert!(policy.explicit); + } + + #[test] + fn builtin_short_alias_inherits_long_command_policy() { + let mut configured = BTreeMap::new(); + configured.insert( + "build".to_string(), + CommandTargetCapability { targets: false }, + ); + let policy = configured_target_policy(Some("b"), &configured); + assert!(!policy.enabled); + assert!(policy.explicit); + } + + #[test] + fn builtin_short_alias_exact_policy_wins_over_long_command_policy() { + let mut configured = BTreeMap::new(); + configured.insert( + "build".to_string(), + CommandTargetCapability { targets: false }, + ); + configured.insert("b".to_string(), CommandTargetCapability { targets: true }); + + let policy = configured_target_policy(Some("b"), &configured); + assert!(policy.enabled); + assert!(policy.explicit); } } diff --git a/src/config.rs b/src/config.rs index a46dd51..f61ca5a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -197,11 +197,11 @@ pub struct WorkspaceConfig { /// packages participate for one already-selected target. #[serde(default)] pub target: BTreeMap, - /// Per-subcommand target-capability opt-in for aliases and custom commands. + /// Per-subcommand configured-target policy. /// - /// Built-in Cargo subcommands get their target capability from code and - /// ignore this table. Unknown aliases (e.g. `lint`) are denied configured - /// targets unless listed here with `targets = true`. + /// Built-in Cargo subcommands default to their code-provided capability. + /// Unknown aliases (e.g. `lint`) default to denied. Entries in this table + /// override either default with `targets = true` or `targets = false`. #[serde(default)] pub subcommands: BTreeMap, } @@ -217,11 +217,11 @@ pub struct WorkspaceTargetOverride { pub exclude_packages: Option, } -/// Workspace-level target-capability opt-in for a single command token. +/// Workspace-level configured-target policy for a single command token. /// -/// A plain `bool` (default `false`) is enough in v1: only unknown aliases -/// consult this table and the default is deny, so "omitted" and -/// `targets = false` are indistinguishable in practice. +/// A plain `bool` is enough in v1. Unknown aliases default to deny, while +/// built-ins default according to cargo-fc's registry. A present +/// `targets = false` entry is therefore an explicit command-level opt-out. #[derive(Serialize, Deserialize, Default, Debug, Clone)] pub struct CommandTargetCapability { /// When `true`, cargo-fc may expand configured target lists and inject diff --git a/src/lib.rs b/src/lib.rs index 8fde08a..a4b44fd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -346,7 +346,8 @@ fn resolve_capability_and_warn( } let token = cli::cargo_subcommand_token(cargo_args); - if cli::command_allows_configured_targets(token.as_deref(), &ws_config.subcommands, ws_key) { + let policy = cli::configured_target_policy(token.as_deref(), &ws_config.subcommands); + if policy.enabled { return true; } @@ -356,7 +357,10 @@ fn resolve_capability_and_warn( || selected .iter() .any(|s| s.config.targets.as_ref().is_some_and(|t| !t.is_empty())); - if has_raw_configured_targets && let Some(token) = token.as_deref().filter(|t| !t.is_empty()) { + if has_raw_configured_targets + && !policy.explicit + && let Some(token) = token.as_deref().filter(|t| !t.is_empty()) + { print_warning!( "not passing configured targets to cargo command `{token}` because it has no targets capability" ); @@ -515,6 +519,24 @@ mod test { )); } + #[test] + fn builtin_command_can_be_disabled_by_workspace_policy() { + let options = Options::default(); + let mut ws = config::WorkspaceConfig::default(); + ws.subcommands.insert( + "build".to_string(), + config::CommandTargetCapability { targets: false }, + ); + + assert!(!resolve_capability_and_warn( + &options, + &["build"], + &ws, + DEFAULT_METADATA_KEY, + &[] + )); + } + #[test] fn no_targets_flag_denies_capability_for_matrix() { let options = Options { From cc99a48902ae56cb7965a654843aa31dc1d9d629 Mon Sep 17 00:00:00 2001 From: romnn Date: Mon, 29 Jun 2026 01:47:59 +0200 Subject: [PATCH 11/13] refactor: clarify target config field names --- plan/per-target-feature-combinations.md | 25 +++++++------- src/config.rs | 26 +++++++-------- src/config/resolve.rs | 43 +++++++++++++------------ src/lib.rs | 15 +++++---- src/target_plan.rs | 27 +++++++++------- tests/metadata_key_aliases.rs | 12 +++++-- 6 files changed, 84 insertions(+), 64 deletions(-) diff --git a/plan/per-target-feature-combinations.md b/plan/per-target-feature-combinations.md index e3d9390..74f043f 100644 --- a/plan/per-target-feature-combinations.md +++ b/plan/per-target-feature-combinations.md @@ -87,7 +87,7 @@ Relevant current structure: - `src/config.rs` - `Config` stores per-package cargo-fc config. - - `Config::target` stores cfg-keyed target override sections. + - `Config::target_overrides` stores cfg-keyed target override sections. - `WorkspaceConfig` currently stores workspace-wide `exclude_packages`. - `src/workspace.rs` - Reads `[workspace.metadata.cargo-fc]`. @@ -580,12 +580,12 @@ Add fields to `WorkspaceConfig`: ```rust pub struct WorkspaceConfig { pub exclude_packages: HashSet, - #[serde(default)] - pub targets: Vec, - #[serde(default)] - pub target: BTreeMap, - #[serde(default)] - pub subcommands: BTreeMap, + #[serde(default, rename = "targets")] + pub workspace_targets: Vec, + #[serde(default, rename = "target")] + pub target_overrides: BTreeMap, + #[serde(default, rename = "subcommands")] + pub subcommand_overrides: BTreeMap, } #[derive(Debug, Clone, Default, Deserialize, Serialize)] @@ -741,8 +741,8 @@ Add package-level targets to `Config`: ```rust pub struct Config { - #[serde(default)] - pub targets: Option>, + #[serde(default, rename = "targets")] + pub package_targets: Option>, // existing fields... } ``` @@ -1399,9 +1399,10 @@ are keyed; planning, config resolution, and feature generation are identical. ### Milestone 1: Config and Target Planning -- Add `WorkspaceConfig::targets`. -- Add `WorkspaceConfig::target` with `WorkspaceTargetOverride`. -- Add `Config::targets: Option>`. +- Add `WorkspaceConfig::workspace_targets` (`targets` in TOML). +- Add `WorkspaceConfig::target_overrides` (`target` in TOML) with + `WorkspaceTargetOverride`. +- Add `Config::package_targets: Option>` (`targets` in TOML). - Add workspace `subcommands..targets` for alias target opt-in. - Add target-source domain types. - Add raw cargo subcommand token extraction for capability lookup. diff --git a/src/config.rs b/src/config.rs index f61ca5a..17cb9c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -31,8 +31,8 @@ pub struct Config { /// /// `targets` is never read by feature-combination generation. Target /// override sections (`target.'cfg(...)'`) must not change it. - #[serde(default)] - pub targets: Option>, + #[serde(default, rename = "targets")] + pub package_targets: Option>, /// Feature sets that must be tested in isolation. #[serde(default)] pub isolated_feature_sets: Vec>, @@ -96,11 +96,11 @@ pub struct Config { #[serde(default)] pub matrix: HashMap, - /// Target-specific configuration overrides. + /// Target-specific package configuration overrides. /// /// This is read from `[package.metadata.cargo-fc.target.'cfg(...)']`. - #[serde(default)] - pub target: BTreeMap, + #[serde(default, rename = "target")] + pub target_overrides: BTreeMap, /// Deprecated configuration keys (kept for backwards compatibility). #[serde(flatten)] pub deprecated: DeprecatedConfig, @@ -109,7 +109,7 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { - targets: None, + package_targets: None, isolated_feature_sets: Vec::new(), exclude_features: HashSet::new(), include_features: HashSet::new(), @@ -123,7 +123,7 @@ impl Default for Config { prune_implied: true, show_pruned: false, matrix: HashMap::new(), - target: BTreeMap::new(), + target_overrides: BTreeMap::new(), deprecated: DeprecatedConfig::default(), } } @@ -189,21 +189,21 @@ pub struct WorkspaceConfig { /// An empty list means "no configured target list"; behavior falls back to /// the existing single effective target detection path. Package-level /// `targets` override (do not merge with) this list. - #[serde(default)] - pub targets: Vec, + #[serde(default, rename = "targets")] + pub workspace_targets: Vec, /// Target-specific workspace overrides keyed by Cargo-style cfg expressions. /// /// These may patch `exclude_packages` only; they select which workspace /// packages participate for one already-selected target. - #[serde(default)] - pub target: BTreeMap, + #[serde(default, rename = "target")] + pub target_overrides: BTreeMap, /// Per-subcommand configured-target policy. /// /// Built-in Cargo subcommands default to their code-provided capability. /// Unknown aliases (e.g. `lint`) default to denied. Entries in this table /// override either default with `targets = true` or `targets = false`. - #[serde(default)] - pub subcommands: BTreeMap, + #[serde(default, rename = "subcommands")] + pub subcommand_overrides: BTreeMap, } /// Target-specific workspace override. diff --git a/src/config/resolve.rs b/src/config/resolve.rs index 5588f2f..dc9af7b 100644 --- a/src/config/resolve.rs +++ b/src/config/resolve.rs @@ -33,10 +33,10 @@ pub fn resolve_config( // Fast path: no matching overrides. if matched.is_empty() { let mut out = base.clone(); - out.target.clear(); + out.target_overrides.clear(); // `targets` is a selection field consumed before resolution; clear it so // the resolved (feature-matrix) config never carries it. - out.targets = None; + out.package_targets = None; return Ok(out); } @@ -73,11 +73,11 @@ pub fn resolve_config( apply_overrides(&mut out, &matched)?; // Remove target metadata from the resolved config. - out.target.clear(); + out.target_overrides.clear(); out.deprecated = DeprecatedConfig::default(); // `targets` is a selection field consumed before resolution; clear it so the // resolved (feature-matrix) config never carries it. - out.targets = None; + out.package_targets = None; Ok(out) } @@ -88,7 +88,7 @@ fn matching_overrides<'a, E: CfgEvaluator>( evaluator: &mut E, ) -> eyre::Result> { let mut matched = Vec::new(); - for (expr, ov) in &base.target { + for (expr, ov) in &base.target_overrides { let is_match = evaluator .matches(expr, target) .wrap_err_with(|| format!("failed to evaluate cfg expression `{expr}`"))?; @@ -498,7 +498,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches @@ -525,7 +525,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches @@ -559,7 +559,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -593,7 +593,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -629,7 +629,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -667,7 +667,10 @@ mod test { let out = resolve_config(&base, &TargetTriple("x".to_string()), &mut eval)?; assert_eq!(out.exclude_features, hs(&["default"])); assert!(out.skip_optional_dependencies); - assert!(out.target.is_empty(), "target metadata should be cleared"); + assert!( + out.target_overrides.is_empty(), + "target metadata should be cleared" + ); Ok(()) } @@ -690,7 +693,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches @@ -736,7 +739,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -769,7 +772,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -795,7 +798,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -818,7 +821,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -847,7 +850,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -879,7 +882,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -908,7 +911,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); @@ -949,7 +952,7 @@ mod test { ..TargetOverride::default() }, ); - base.target = target; + base.target_overrides = target; let mut eval = StubEval::default(); eval.matches.insert("cfg(unix)".to_string()); diff --git a/src/lib.rs b/src/lib.rs index a4b44fd..a4ac55b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -346,17 +346,20 @@ fn resolve_capability_and_warn( } let token = cli::cargo_subcommand_token(cargo_args); - let policy = cli::configured_target_policy(token.as_deref(), &ws_config.subcommands); + let policy = cli::configured_target_policy(token.as_deref(), &ws_config.subcommand_overrides); if policy.enabled { return true; } // Capability denied: warn (once) only when the user actually configured // targets that we are now skipping. - let has_raw_configured_targets = !ws_config.targets.is_empty() - || selected - .iter() - .any(|s| s.config.targets.as_ref().is_some_and(|t| !t.is_empty())); + let has_raw_configured_targets = !ws_config.workspace_targets.is_empty() + || selected.iter().any(|s| { + s.config + .package_targets + .as_ref() + .is_some_and(|t| !t.is_empty()) + }); if has_raw_configured_targets && !policy.explicit && let Some(token) = token.as_deref().filter(|t| !t.is_empty()) @@ -523,7 +526,7 @@ mod test { fn builtin_command_can_be_disabled_by_workspace_policy() { let options = Options::default(); let mut ws = config::WorkspaceConfig::default(); - ws.subcommands.insert( + ws.subcommand_overrides.insert( "build".to_string(), config::CommandTargetCapability { targets: false }, ); diff --git a/src/target_plan.rs b/src/target_plan.rs index fd58169..fa43ee9 100644 --- a/src/target_plan.rs +++ b/src/target_plan.rs @@ -118,7 +118,7 @@ fn package_target_list( env: &impl TargetEnvironment, fallback_cache: &mut Option, ) -> eyre::Result> { - match &selected.config.targets { + match &selected.config.package_targets { // Package-level list present. Some(list) if !list.is_empty() => { let triples = normalize_targets(list)?; @@ -236,7 +236,8 @@ pub fn build_target_plans<'a>( let contains_configured_assignments = if cli_target.is_some() { true } else if capability_allowed { - !workspace_config.targets.is_empty() || selected.iter().any(|s| s.config.targets.is_some()) + !workspace_config.workspace_targets.is_empty() + || selected.iter().any(|s| s.config.package_targets.is_some()) } else { false }; @@ -263,7 +264,7 @@ pub fn build_target_plans<'a>( }) .collect() } else if capability_allowed { - let workspace_targets = normalize_targets(&workspace_config.targets)?; + let workspace_targets = normalize_targets(&workspace_config.workspace_targets)?; let mut out = Vec::with_capacity(selected.len()); for s in selected { let targets = package_target_list(s, &workspace_targets, env, &mut fallback_cache)?; @@ -295,7 +296,7 @@ pub fn build_target_plans<'a>( let mut seen: HashSet = HashSet::new(); if cli_target.is_none() && capability_allowed { - for triple in normalize_targets(&workspace_config.targets)? { + for triple in normalize_targets(&workspace_config.workspace_targets)? { if seen.insert(triple.clone()) { order.push(triple); } @@ -314,8 +315,12 @@ pub fn build_target_plans<'a>( // exclude set for that target, and drop empty plans. let mut plans = Vec::new(); for triple in &order { - let exclude = - resolve_target_excludes(base_exclude, &workspace_config.target, triple, evaluator)?; + let exclude = resolve_target_excludes( + base_exclude, + &workspace_config.target_overrides, + triple, + evaluator, + )?; let mut packages = Vec::new(); for pt in &package_targets { if exclude.contains(pt.package.name.as_str()) { @@ -418,14 +423,14 @@ mod test { fn config_with_targets(targets: Option<&[&str]>) -> Config { Config { - targets: targets.map(|t| t.iter().map(|s| (*s).to_string()).collect()), + package_targets: targets.map(|t| t.iter().map(|s| (*s).to_string()).collect()), ..Config::default() } } fn workspace_targets(targets: &[&str]) -> WorkspaceConfig { WorkspaceConfig { - targets: targets.iter().map(|s| (*s).to_string()).collect(), + workspace_targets: targets.iter().map(|s| (*s).to_string()).collect(), ..WorkspaceConfig::default() } } @@ -750,8 +755,8 @@ mod test { }, ); let ws = WorkspaceConfig { - targets: vec!["linux".to_string(), "wasm".to_string()], - target, + workspace_targets: vec!["linux".to_string(), "wasm".to_string()], + target_overrides: target, ..WorkspaceConfig::default() }; @@ -807,7 +812,7 @@ mod test { }, ); let ws = WorkspaceConfig { - target, + target_overrides: target, ..WorkspaceConfig::default() }; diff --git a/tests/metadata_key_aliases.rs b/tests/metadata_key_aliases.rs index 3d5a33b..daf12cb 100644 --- a/tests/metadata_key_aliases.rs +++ b/tests/metadata_key_aliases.rs @@ -114,7 +114,11 @@ fn alias_cargo_fc_with_target_override() -> eyre::Result<()> { assert!(config.exclude_features.contains("foo")); // Target overrides are stored in the config but not applied until resolve_config - assert!(config.target.contains_key("cfg(target_os = \"linux\")")); + assert!( + config + .target_overrides + .contains_key("cfg(target_os = \"linux\")") + ); Ok(()) } @@ -129,7 +133,11 @@ fn alias_fc_with_target_override() -> eyre::Result<()> { "#})?; assert!(config.exclude_features.contains("foo")); - assert!(config.target.contains_key("cfg(target_os = \"linux\")")); + assert!( + config + .target_overrides + .contains_key("cfg(target_os = \"linux\")") + ); Ok(()) } From 15cf51e70abffae098a54a966a3bb6cf207a07b9 Mon Sep 17 00:00:00 2001 From: romnn Date: Mon, 29 Jun 2026 03:10:36 +0200 Subject: [PATCH 12/13] refactor(target): remove redundant target detector --- plan/per-target-feature-combinations.md | 5 +- src/cfg_eval.rs | 6 +- src/target.rs | 176 +----------------------- 3 files changed, 7 insertions(+), 180 deletions(-) diff --git a/plan/per-target-feature-combinations.md b/plan/per-target-feature-combinations.md index 74f043f..5e69a73 100644 --- a/plan/per-target-feature-combinations.md +++ b/plan/per-target-feature-combinations.md @@ -253,8 +253,9 @@ pub struct SelectedPackage<'a> { The exact trait names can differ. The point is that target planning should be unit-testable without invoking real `rustc` or spawning Cargo. The production -adapter can delegate to the existing `RustcTargetDetector`, -`RustcCfgEvaluator`, and one package config loading pass. +adapter can read `CARGO_BUILD_TARGET`, delegate host detection to `rustc -vV`, +use `RustcCfgEvaluator` for cfg matching, and share one package config loading +pass. Planning must also receive a `CfgEvaluator`. Target-specific workspace `exclude_packages` uses `cfg(...)` keys, so planning needs the same target cfg diff --git a/src/cfg_eval.rs b/src/cfg_eval.rs index 274a6d2..f1730a5 100644 --- a/src/cfg_eval.rs +++ b/src/cfg_eval.rs @@ -185,13 +185,13 @@ impl CfgEvaluator for RustcCfgEvaluator { #[cfg(test)] mod test { use super::{CfgEvaluator, RustcCfgEvaluator}; - use crate::target::TargetDetector; + use crate::target::host_triple; use color_eyre::eyre; #[test] fn matches_simple_true_for_target_arch() -> eyre::Result<()> { let mut eval = RustcCfgEvaluator::default(); - let host = crate::target::RustcTargetDetector::default().detect_target(&Vec::new())?; + let host = host_triple()?; // Host must match its own arch. let cfg_set = std::process::Command::new("rustc") @@ -217,7 +217,7 @@ mod test { #[test] fn rejects_feature_predicate() -> eyre::Result<()> { let mut eval = RustcCfgEvaluator::default(); - let host = crate::target::RustcTargetDetector::default().detect_target(&Vec::new())?; + let host = host_triple()?; let err = match eval.matches(r#"cfg(feature = "foo")"#, &host) { Ok(v) => eyre::bail!("expected cfg(feature=...) to be rejected, got {v}"), diff --git a/src/target.rs b/src/target.rs index 5e8a5df..731a584 100644 --- a/src/target.rs +++ b/src/target.rs @@ -147,84 +147,9 @@ pub fn parse_cli_target(cargo_args: &[String]) -> Option { None } -/// Read-only access to environment variables. -/// -/// This trait abstracts environment variable lookups so that production code -/// uses the real process environment while tests can supply deterministic -/// values without data races. -pub trait Env { - /// Look up an environment variable by name. - fn var(&self, key: &str) -> Option; -} - -/// The real process environment (delegates to [`std::env::var`]). -#[derive(Debug, Default, Clone, Copy)] -pub struct ProcessEnv; - -impl Env for ProcessEnv { - fn var(&self, key: &str) -> Option { - std::env::var(key).ok() - } -} - -/// Detect the effective compilation target for this invocation. -pub trait TargetDetector { - /// Determine the effective target triple. - /// - /// # Errors - /// - /// Returns an error if the target triple cannot be determined. - fn detect_target(&self, cargo_args: &[String]) -> eyre::Result; -} - -/// Detect the compilation target using CLI flags, environment, and `rustc`. -/// -/// Resolution order: -/// 1. `--target ` CLI flag (authoritative) -/// 2. `CARGO_BUILD_TARGET` environment variable -/// 3. Host target via `rustc -vV` -#[derive(Debug, Clone)] -pub struct RustcTargetDetector { - env: E, -} - -impl Default for RustcTargetDetector { - fn default() -> Self { - Self { env: ProcessEnv } - } -} - -impl RustcTargetDetector { - /// Create a detector with a custom environment. - pub fn with_env(env: E) -> Self { - Self { env } - } - - fn parse_target_flag(cargo_args: &[String]) -> Option { - parse_cli_target(cargo_args) - } -} - -impl TargetDetector for RustcTargetDetector { - fn detect_target(&self, cargo_args: &[String]) -> eyre::Result { - if let Some(triple) = Self::parse_target_flag(cargo_args) { - return Ok(TargetTriple(triple)); - } - if let Some(triple) = self.env.var("CARGO_BUILD_TARGET") { - let triple = triple.trim(); - if !triple.is_empty() { - return Ok(TargetTriple(triple.to_string())); - } - } - host_triple() - } -} - #[cfg(test)] mod test { - use super::{Env, RustcTargetDetector, TargetDetector, parse_cli_target}; - use color_eyre::eyre; - use std::collections::HashMap; + use super::parse_cli_target; fn owned(values: &[&str]) -> Vec { values.iter().copied().map(String::from).collect() @@ -275,103 +200,4 @@ mod test { fn parse_cli_target_absent() { assert_eq!(parse_cli_target(&owned(&["check", "--all-features"])), None); } - - #[derive(Default)] - struct TestEnv { - vars: HashMap, - } - - impl Env for TestEnv { - fn var(&self, key: &str) -> Option { - self.vars.get(key).cloned() - } - } - - #[test] - fn parses_target_flag_separate_value() -> eyre::Result<()> { - let d = RustcTargetDetector::default(); - let args = vec!["--target".to_string(), "wasm32-unknown-unknown".to_string()]; - let triple = d.detect_target(&args)?; - assert_eq!(triple.as_str(), "wasm32-unknown-unknown"); - Ok(()) - } - - #[test] - fn parses_target_flag_equals_form() -> eyre::Result<()> { - let d = RustcTargetDetector::default(); - let args = vec!["--target=wasm32-unknown-unknown".to_string()]; - let triple = d.detect_target(&args)?; - assert_eq!(triple.as_str(), "wasm32-unknown-unknown"); - Ok(()) - } - - #[test] - fn respects_cargo_build_target_env_var() -> eyre::Result<()> { - let env = TestEnv { - vars: HashMap::from([( - "CARGO_BUILD_TARGET".to_string(), - "aarch64-unknown-linux-gnu".to_string(), - )]), - }; - let d = RustcTargetDetector::with_env(env); - let triple = d.detect_target(&Vec::new())?; - assert_eq!(triple.as_str(), "aarch64-unknown-linux-gnu"); - Ok(()) - } - - #[test] - fn cli_target_takes_precedence_over_env_var() -> eyre::Result<()> { - let env = TestEnv { - vars: HashMap::from([( - "CARGO_BUILD_TARGET".to_string(), - "aarch64-unknown-linux-gnu".to_string(), - )]), - }; - let d = RustcTargetDetector::with_env(env); - let args = vec!["--target".to_string(), "wasm32-unknown-unknown".to_string()]; - let triple = d.detect_target(&args)?; - assert_eq!(triple.as_str(), "wasm32-unknown-unknown"); - Ok(()) - } - - #[test] - fn detector_ignores_target_flag_after_double_dash() -> eyre::Result<()> { - let env = TestEnv { - vars: HashMap::from([( - "CARGO_BUILD_TARGET".to_string(), - "aarch64-unknown-linux-gnu".to_string(), - )]), - }; - let d = RustcTargetDetector::with_env(env); - let args = vec![ - "run".to_string(), - "--".to_string(), - "--target".to_string(), - "binary-arg".to_string(), - ]; - let triple = d.detect_target(&args)?; - assert_eq!(triple.as_str(), "aarch64-unknown-linux-gnu"); - Ok(()) - } - - #[test] - fn empty_env_var_falls_through_to_host() -> eyre::Result<()> { - let env = TestEnv { - vars: HashMap::from([("CARGO_BUILD_TARGET".to_string(), " ".to_string())]), - }; - let d = RustcTargetDetector::with_env(env); - // No --target flag, empty env var → should fall through to host triple. - let triple = d.detect_target(&Vec::new())?; - assert!(!triple.as_str().is_empty()); - Ok(()) - } - - #[test] - fn missing_env_var_falls_through_to_host() -> eyre::Result<()> { - let env = TestEnv::default(); - let d = RustcTargetDetector::with_env(env); - let triple = d.detect_target(&Vec::new())?; - assert!(!triple.as_str().is_empty()); - Ok(()) - } } From 3d382c830b2686bf76bbde928f28feecd5041fc6 Mon Sep 17 00:00:00 2001 From: romnn Date: Mon, 29 Jun 2026 03:11:08 +0200 Subject: [PATCH 13/13] refactor(runner): streamline target plan adapters --- src/runner.rs | 198 ++++++++++++++++++++++----------------------- src/target_plan.rs | 17 ++-- 2 files changed, 107 insertions(+), 108 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index ed3cecd..2b2f01e 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -529,11 +529,44 @@ pub struct MatrixOptions { pub no_prune_implied: bool, } +fn load_configs_for_packages(packages: &[&cargo_metadata::Package]) -> eyre::Result> { + packages + .iter() + .map(|package| package.config()) + .collect::>>() +} + +fn single_target_plans<'a>( + packages: &[&'a cargo_metadata::Package], + configs: &'a [Config], + target: &TargetTriple, +) -> TargetPlans<'a> { + let effective_target = EffectiveTarget { + triple: target.clone(), + source: TargetSource::Cli, + }; + + TargetPlans { + plans: vec![TargetPlan { + target: target.clone(), + packages: packages + .iter() + .zip(configs) + .map(|(package, config)| PlannedPackage { + package, + config, + target: effective_target.clone(), + }) + .collect(), + }], + contains_configured_assignments: false, + } +} + /// Print a JSON feature matrix for the given packages to stdout. /// -/// The matrix is a JSON array of objects produced from each package's -/// configuration and the feature combinations returned by -/// [`Package::feature_matrix`]. +/// The matrix is a JSON array of objects produced from each package's resolved +/// target-specific configuration and feature combinations. /// /// # Errors /// @@ -545,56 +578,19 @@ pub fn print_feature_matrix_for_target( evaluator: &mut impl crate::cfg_eval::CfgEvaluator, options: &MatrixOptions, ) -> eyre::Result { - let per_package_features = packages - .iter() - .map(|pkg| { - let base_config = pkg.config()?; - let config = crate::config::resolve::resolve_config(&base_config, target, evaluator)?; - let features = if options.packages_only { - vec!["default".to_string()] - } else { - let combos = pkg.feature_combinations(&config)?; - let combos = crate::implication::maybe_prune( - combos, - &pkg.features, - &config, - options.no_prune_implied, - ); - combos - .keep - .into_iter() - .map(|combo| combo.into_iter().join(",")) - .collect() - }; - Ok::<_, eyre::Report>((pkg.name.clone(), config, features)) - }) - .collect::, _>>()?; - - let matrix: Vec = per_package_features - .into_iter() - .flat_map(|(name, config, features)| { - let target = target.as_str().to_string(); - features.into_iter().map(move |ft| { - use serde_json_merge::{iter::dfs::Dfs, merge::Merge}; - - let mut out = serde_json::json!(config.matrix); - out.merge::(&serde_json::json!({ - "name": name, - "target": target, - "features": ft, - })); - out - }) - }) - .collect(); - - let matrix = if options.pretty { - serde_json::to_string_pretty(&matrix) - } else { - serde_json::to_string(&matrix) - }?; - println!("{matrix}"); - Ok(None) + let configs = load_configs_for_packages(packages)?; + let target_plans = single_target_plans(packages, &configs, target); + let build_options = Options { + no_prune_implied: options.no_prune_implied, + ..Options::default() + }; + let plan_set = build_execution_plans( + &target_plans, + &build_options, + options.packages_only, + evaluator, + )?; + print_matrix_for_execution_plans(&plan_set, options) } /// A resolved, owned execution plan for one concrete target. @@ -1023,30 +1019,8 @@ pub fn run_cargo_command_for_target( target: &TargetTriple, evaluator: &mut impl CfgEvaluator, ) -> eyre::Result { - let configs: Vec = packages - .iter() - .map(|package| package.config()) - .collect::>>()?; - - let target_plans = TargetPlans { - plans: vec![TargetPlan { - target: target.clone(), - packages: packages - .iter() - .zip(&configs) - .map(|(package, config)| PlannedPackage { - package, - config, - target: EffectiveTarget { - triple: target.clone(), - source: TargetSource::Cli, - }, - }) - .collect(), - }], - contains_configured_assignments: false, - }; - + let configs = load_configs_for_packages(packages)?; + let target_plans = single_target_plans(packages, &configs, target); let plan_set = build_execution_plans(&target_plans, options, false, evaluator)?; run_execution_plans( &plan_set, @@ -1198,18 +1172,14 @@ fn execute_serial( seen_diagnostics, stdout, )?; - let should_stop = ctx.options.fail_fast && !result.summary.pedantic_success; - let exit_code = result.summary.exit_code; - summary.push(result.summary); - if should_stop { - if ctx.options.summary_only { - io::copy(&mut io::Cursor::new(result.colored_output), stdout)?; - stdout.flush().ok(); - } - let code = - print_summary(&summary, plan_set.show_pruned, stdout, start.elapsed()) - .or(exit_code) - .unwrap_or(1); + if let Some(code) = record_result_and_maybe_stop( + &mut summary, + result, + plan_set.show_pruned, + ctx, + stdout, + start, + )? { return Ok(Some(code)); } } @@ -1277,17 +1247,14 @@ fn execute_aggregate( seen_diagnostics, stdout, )?; - let should_stop = ctx.options.fail_fast && !result.summary.pedantic_success; - let exit_code = result.summary.exit_code; - summary.push(result.summary); - if should_stop { - if ctx.options.summary_only { - io::copy(&mut io::Cursor::new(result.colored_output), stdout)?; - stdout.flush().ok(); - } - let code = print_summary(&summary, plan_set.show_pruned, stdout, start.elapsed()) - .or(exit_code) - .unwrap_or(1); + if let Some(code) = record_result_and_maybe_stop( + &mut summary, + result, + plan_set.show_pruned, + ctx, + stdout, + start, + )? { return Ok(Some(code)); } } @@ -1300,6 +1267,37 @@ fn execute_aggregate( )) } +fn record_result_and_maybe_stop( + summary: &mut Vec, + result: CombinationResult, + show_pruned: bool, + ctx: &RunContext<'_>, + stdout: &mut StandardStream, + start: Instant, +) -> eyre::Result { + let CombinationResult { + summary: result_summary, + colored_output, + } = result; + let should_stop = ctx.options.fail_fast && !result_summary.pedantic_success; + let exit_code = result_summary.exit_code; + summary.push(result_summary); + + if !should_stop { + return Ok(None); + } + + if ctx.options.summary_only { + io::copy(&mut io::Cursor::new(colored_output), stdout)?; + stdout.flush().ok(); + } + Ok(Some( + print_summary(summary, show_pruned, stdout, start.elapsed()) + .or(exit_code) + .unwrap_or(1), + )) +} + /// Transpose per-target execution plans into aggregate-mode invocations. /// /// The resulting order is package first-appearance order, sorted canonical combo diff --git a/src/target_plan.rs b/src/target_plan.rs index fa43ee9..51697c4 100644 --- a/src/target_plan.rs +++ b/src/target_plan.rs @@ -232,12 +232,16 @@ pub fn build_target_plans<'a>( evaluator: &mut impl CfgEvaluator, ) -> eyre::Result> { let mut fallback_cache: Option = None; + let workspace_targets = if cli_target.is_none() && capability_allowed { + normalize_targets(&workspace_config.workspace_targets)? + } else { + Vec::new() + }; let contains_configured_assignments = if cli_target.is_some() { true } else if capability_allowed { - !workspace_config.workspace_targets.is_empty() - || selected.iter().any(|s| s.config.package_targets.is_some()) + !workspace_targets.is_empty() || selected.iter().any(|s| s.config.package_targets.is_some()) } else { false }; @@ -264,7 +268,6 @@ pub fn build_target_plans<'a>( }) .collect() } else if capability_allowed { - let workspace_targets = normalize_targets(&workspace_config.workspace_targets)?; let mut out = Vec::with_capacity(selected.len()); for s in selected { let targets = package_target_list(s, &workspace_targets, env, &mut fallback_cache)?; @@ -295,11 +298,9 @@ pub fn build_target_plans<'a>( let mut order: Vec = Vec::new(); let mut seen: HashSet = HashSet::new(); - if cli_target.is_none() && capability_allowed { - for triple in normalize_targets(&workspace_config.workspace_targets)? { - if seen.insert(triple.clone()) { - order.push(triple); - } + for triple in workspace_targets { + if seen.insert(triple.clone()) { + order.push(triple); } } for pt in &package_targets {