Repository navigation
feat(cargo): add opt-in workspace target ownership - #235
Merged
Merged
Conversation
Merged
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review on ArchCode
Problem and author intent
Aster deduplicates target addresses. When several Cargo members each define a command that checks their whole workspace, selecting those members launches the same operation repeatedly. Cargo then serializes those processes on its build directory. Keeping the definitions aligned also requires forwarding stanzas in every new crate.
This PR makes workspace ownership explicit and opt-in. Unconfigured workspaces retain their existing commands and dependency edges, and discovery still works without Cargo installed.
What changed
Each listed target needs an explicit command at the Cargo root. Cargo metadata supplies membership; Aster generates dependency-only member entry points pointing to one owner address. Root plus member selections therefore execute the owner once. Conflicting member definitions and ownership cycles are rejected.
depends_onwithoutcommand. They launch no process and store no artifact-cache entry, while preserving failure and dependency-hash propagation.Scope: backend-only Rust CLI, configuration and documentation. Risk: medium; ownership is opt-in, but discovery, caching and dependency-only scheduling interact. Incorrect inputs could skip a required gate. Existing aliases and non-opted-in discovery have compatibility tests.
User impact: opted-in repositories can define shared Cargo gates once. Commands run as written; Aster does not add
--workspaceor change profiles. This PR does not enable the feature in downstream repositories.Verification
Canonical process proof: tests/cli_tests/cargo_workspace.rs,
cargo_workspace_members_share_gates_and_invalidate_unselected_inputs. Real Aster, Cargo, Git and compiler processes run a temporary workspace. Assertions cover one owner execution, warm-cache reuse, member/asset invalidation, automatic membership, independent package tests and variants, aliases, and propagation of a real compiler failure.affectednested_cargo_workspace_hashes_external_modules_without_claiming_excluded_cratesunconfigured_cargo_workspaces_keep_package_targets_without_invoking_cargocargo_workspace_rejects_ambiguous_or_cyclic_ownership_before_executiondependency_only_targets_follow_prerequisite_results_without_own_cache_entriesFidelity: production discovery, scheduler, cache and affected selection; real Cargo metadata/build/test and Git. Shell wrappers only record actual gate invocations. No admission or input-selection logic is mocked.
Local results: 208 focused unit tests and all five integration tests pass. Formatting, workspace-wide Clippy with warnings denied, and documentation checks pass. Three separately compiled mutations fail the intended assertions: omitted generated affected-input selection, omitted member-file hashing, and omitted transitive dependency hashes. Independent code and test review completed.
CI result: all 13 checks pass at
e4fe4baa491cf0d37bf68877aaaf751bcf69a4c2, including all five new tests on both Linux and macOS, dependency audit, MSRV, quality, CodeQL, packaging and the server container smoke test. ArchDev reported no rule violations. Its medium risk assessment identifies residual cache/discovery complexity and Unix-only process coverage, without a specific defect.Automatic coverage: existing
CIworkflow on pull requests/main pushes. Linux runscargo test --workspace --all-targets --all-features; macOS explicitly includes--test integration. The added module is discovered throughtests/integration.rs -> cli_tests/mod.rs. Exhaustive tests and dependency audit run in CI.Not proven here / limits: Windows runtime is outside these Unix fixtures. Cross-invocation serialization is unchanged. Inputs read by arbitrary build scripts outside member/custom source directories still require explicit cache declarations. Direct streaming of commandless targets is rejected; use
--no-stream. Downstream configuration adoption and release are separate from this PR.Checklist