diff --git a/Cargo.lock b/Cargo.lock index e17731ee..130c97d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1731,6 +1731,15 @@ dependencies = [ "sha2", ] +[[package]] +name = "matchers" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" +dependencies = [ + "regex-automata", +] + [[package]] name = "maybe-owned" version = "0.3.4" @@ -1938,6 +1947,15 @@ dependencies = [ "whitaker-common", ] +[[package]] +name = "nu-ansi-term" +version = "0.50.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" +dependencies = [ + "windows-sys 0.61.2", +] + [[package]] name = "num-conv" version = "0.2.2" @@ -2864,6 +2882,15 @@ dependencies = [ "digest", ] +[[package]] +name = "sharded-slab" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" +dependencies = [ + "lazy_static", +] + [[package]] name = "shlex" version = "2.0.1" @@ -3143,6 +3170,15 @@ dependencies = [ "syn 3.0.3", ] +[[package]] +name = "thread_local" +version = "1.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ad99c4c6d32803332c548b1af0540b357b3f5fc0be8f6c6bfe8b2e6ae784070" +dependencies = [ + "cfg-if", +] + [[package]] name = "time" version = "0.3.53" @@ -3284,6 +3320,36 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "db97caf9d906fbde555dd62fa95ddba9eecfd14cb388e4f491a66d74cd5fb79a" dependencies = [ "once_cell", + "valuable", +] + +[[package]] +name = "tracing-log" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee855f1f400bd0e5c02d150ae5de3840039a3f54b025156404e34c23c03f47c3" +dependencies = [ + "log", + "once_cell", + "tracing-core", +] + +[[package]] +name = "tracing-subscriber" +version = "0.3.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb7f578e5945fb242538965c2d0b04418d38ec25c79d160cd279bf0731c8d319" +dependencies = [ + "matchers", + "nu-ansi-term", + "once_cell", + "regex-automata", + "sharded-slab", + "smallvec", + "thread_local", + "tracing", + "tracing-core", + "tracing-log", ] [[package]] @@ -3505,6 +3571,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "valuable" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" + [[package]] name = "value-bag" version = "1.13.0" @@ -3664,9 +3736,11 @@ dependencies = [ "directories-next", "flate2", "fs2", + "insta", "libc", "log", "mockall", + "proptest", "rstest", "rstest-bdd", "rstest-bdd-macros", @@ -3679,6 +3753,7 @@ dependencies = [ "thiserror 2.0.19", "toml 1.1.3+spec-1.1.0", "tracing", + "tracing-subscriber", "trybuild", "ureq", "wait-timeout", diff --git a/README.md b/README.md index b3c06533..74d865d4 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,10 @@ suite flows opt in with `--experimental` or the corresponding suite feature. - **Configurable thresholds** — Adjust limits like `module_max_lines.max_lines` to match your team's standards. - **Modular design** — Use individual lints or load the whole suite. +- **Pinnable suite** — The standalone installer's `--ref ` builds and + stages the lint suite from an exact commit of `leynos/whitaker`, for + reproducible lint behaviour across CI. See + [Pinning the suite](docs/users-guide.md#pinning-the-suite). ## Project Status diff --git a/crates/no_std_fs_operations/src/lib.rs b/crates/no_std_fs_operations/src/lib.rs index 0be3f30c..7af853cb 100644 --- a/crates/no_std_fs_operations/src/lib.rs +++ b/crates/no_std_fs_operations/src/lib.rs @@ -22,15 +22,3 @@ mod usage; pub use config::NoStdFsConfig; #[cfg(feature = "dylint-driver")] pub use driver::*; - -#[cfg(not(feature = "dylint-driver"))] -mod stub { - //! Placeholder compiled when the `dylint-driver` feature is off, so the - //! crate still builds without the `rustc_private` toolchain internals. - - #[expect( - dead_code, - reason = "Exposed only when built without the `dylint-driver` feature" - )] - pub fn no_std_fs_operations_disabled_stub() {} -} diff --git a/crates/no_std_fs_operations/tests/no_default_features_build.rs b/crates/no_std_fs_operations/tests/no_default_features_build.rs new file mode 100644 index 00000000..181ed293 --- /dev/null +++ b/crates/no_std_fs_operations/tests/no_default_features_build.rs @@ -0,0 +1,90 @@ +//! Compile-time regression guard for the `dylint-driver`-disabled build. +//! +//! The crate is a Dylint lint library: the lint logic lives behind the +//! optional `dylint-driver` feature, while the default feature set is empty so +//! the crate can be built by tools that only need a plain library. Issue #322 +//! removed the private stub that previously existed solely to keep that +//! empty build warning-free. This test re-checks the configuration the stub +//! existed for: `cargo check --no-default-features --lib` must still succeed +//! with warnings denied, now without relying on that stub. +//! +//! The check is deliberately restricted to the library target: the crate's +//! integration test binaries are Dylint harnesses that require `cargo-dylint` +//! and `dylint-link`, which must not be assumed here. The nested `cargo` +//! invocation therefore inherits the outer `RUSTFLAGS` (so `-D warnings` from +//! the Makefile gate applies) but uses an isolated target directory so it never +//! contends with the outer build. + +use std::path::PathBuf; +use std::process::Command; + +use anyhow::Context as _; +use serde_json::Value; +use tempfile::TempDir; + +/// Top-level name of the package under test. +const CRATE: &str = "no_std_fs_operations"; + +/// Runs `cargo check --no-default-features --lib` for this crate and asserts +/// that the build succeeds with warnings denied (via inherited `RUSTFLAGS`). +/// +/// The workspace manifest is located by walking up from this test crate's +/// manifest directory, mirroring `integration_exclusion.rs`, so the nested +/// invocation resolves the same workspace this test builds under. +#[test] +fn crate_builds_without_dylint_driver_feature() -> anyhow::Result<()> { + let workspace_root = workspace_root()?; + let target_dir = TempDir::new().context("failed to create isolated target directory")?; + + let output = Command::new("cargo") + .arg("check") + .arg("--package") + .arg(CRATE) + .arg("--no-default-features") + .arg("--lib") + .arg("--message-format=json") + .current_dir(&workspace_root) + .env("CARGO_TARGET_DIR", target_dir.path()) + .output() + .context("failed to execute nested cargo check")?; + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "`cargo check --no-default-features --lib` for `{CRATE}` failed \ + (it must compile without the `dylint-driver` feature now that the \ + no-driver stub is gone):\n{stderr}" + ); + + let diagnostics = stdout + .lines() + .filter_map(|line| serde_json::from_str::(line).ok()) + .filter(|message| message["reason"] == "compiler-message") + .collect::>(); + assert!( + diagnostics.is_empty(), + "`cargo check --no-default-features --lib` for `{CRATE}` emitted \ + compiler diagnostics under `-D warnings`: {diagnostics:#?}" + ); + + Ok(()) +} + +/// Returns the workspace root containing this crate's manifest. +fn workspace_root() -> anyhow::Result { + let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let mut candidate = manifest_dir.as_path(); + loop { + if candidate.join("Cargo.toml").is_file() { + let workspace = std::fs::read_to_string(candidate.join("Cargo.toml")) + .context("failed to read candidate workspace Cargo.toml")?; + if workspace.contains("[workspace]") { + return Ok(candidate.to_path_buf()); + } + } + candidate = candidate + .parent() + .context("workspace root not found above CARGO_MANIFEST_DIR")?; + } +} diff --git a/docs/contents.md b/docs/contents.md index 446be541..92838002 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -19,6 +19,11 @@ - [Roadmap](roadmap.md) tracks planned work, implementation phases, and larger changes that have not yet landed. +## Migration guides + +- [Migrating to 0.3.0](migrations/0.3.0.md) summarizes installer pinning, + rolling-default behaviour, and the workspace rules for pinned checkouts. + ## Primary design documents - [Whitaker Dylint suite design](whitaker-dylint-suite-design.md) explains the diff --git a/docs/developers-guide.md b/docs/developers-guide.md index fb6add6d..a725df5c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1837,6 +1837,7 @@ Whitaker data directory keyed by toolchain and target: - `--skip-deps` — Skip `cargo-dylint`/`dylint-link` installation check - `--skip-wrapper` — Skip wrapper script generation - `--no-update` — Don't update existing repository clone +- `--ref REF` — Pin the suite to a commit-ish (SHA, tag, or branch name) ### Using installed lints @@ -1872,6 +1873,98 @@ This skips building entirely, providing faster lint runs during development. set of focused private helpers. Understanding them is useful when extending the installation pipeline. +#### Private helper boundaries + +Installer helpers remain private to the module that owns their side effects. +`git::run_git_checked` is only for Git commands whose successful output is +discarded and whose non-zero exit status maps directly to `InstallerError::Git`; +commands that inspect output or interpret a non-zero status must continue to use +`run_git_with_timeout` directly. Real-Git regression fixtures and tests belong +in `git_tests.rs`, keeping the production adapter focused. The helpers in +`workspace_progress.rs` only render operator messages at the CLI edge and must +not clone, update, pin, or rediscover the checkout themselves. +`workspace::resolve_workspace_action` owns the environment-dependent action +selection and is shared with dry-run validation. `ensure_workspace` performs +that action and returns it in `WorkspaceCheckout`; `report_workspace_progress` +must render from this recorded action instead of recomputing repository state. +`workspace::finalize_workspace_checkout` is called only from +`ensure_workspace` action arms after each arm's current-directory, clone, or +update setup. It may combine optional pinning with `WorkspaceCheckout` +construction and record the supplied action; it must not select an action or +clone, update, or reattach a repository. + +`workspace::WorkspaceRepository` is a private seam for workspace orchestration. +The production `GitWorkspaceRepository` implementation delegates clone, update, +and default-branch operations to `crate::git`; `ensure_workspace` must compose +through that implementation, keeping Git process ownership in the workspace +boundary. Tests may inject a repository implementation only through the +private preparation helper to exercise action re-evaluation and lock behaviour +without network or process side effects. The seam must not become a public Git +adapter or be used to bypass the `crate::git` APIs in production. + +Behaviour-test support follows the same ownership rule. +`behaviour_cli::support::output_for_assertions` combines scenario-skip handling +with borrowing the captured process output. Assertion-specific expectations +remain in their step functions. Pinned-ref scenario setup and assertions stay +in `support/pinned_ref.rs`; support for other behaviour suites should stay local +unless multiple suites need exactly the same contract. + +#### Public Git operation APIs + +The `whitaker_installer::git` module exposes the following Git operations. They +all accept a UTF-8 repository path and return the installer's `Result` type; +each operation is bounded by the module's five-minute Git timeout. Use these +functions for the managed clone workflow, not to mutate a user's current +Whitaker checkout. + +*Table: Public Git operation APIs.* + +| API | Purpose | Usage constraints | +| --- | --- | --- | +| `clone_repository(target: &Utf8Path) -> Result<()>` | Clones the public Whitaker repository into `target`. | Creates missing parent directories and uses the five-minute Git timeout. Use only for the installer-managed clone. | +| `update_repository(repo: &Utf8Path) -> Result<()>` | Pulls the latest changes into an existing Whitaker clone. | Call `ensure_default_branch` first when a prior pin may have detached `HEAD`; this function does not reattach a checkout. | +| `resolve_commit(repo: &Utf8Path, refspec: &str) -> Result` | Resolves a local commit-ish (SHA, tag, or branch) to a validated full object ID and peels annotated tags. | Does not fetch. Call it when the ref is expected to exist locally, or after `fetch_ref` has populated the clone. | +| `fetch_ref(repo: &Utf8Path, refspec: &str) -> Result` | Fetches the requested ref from `origin` into the private `refs/whitaker/pinned-ref` ref and returns its validated full object ID. | Use it to refresh a requested pin before checkout. It force-updates only the private pin ref; it does not move the current branch or check out the result. | +| `checkout_detached(repo: &Utf8Path, commit: &CommitSha) -> Result<()>` | Checks out exactly `commit` with a detached `HEAD`. | Use only for the installer-managed clone after resolving the requested ref. The workspace layer must reject pinning in the user's current Whitaker workspace first. | +| `detached_head_commit(repo: &Utf8Path) -> Result>` | Returns the full object ID when `HEAD` is detached, or `None` when `HEAD` names a branch. | Use to preserve provenance for an unpinned `--no-update` install that reuses a detached managed clone. | +| `ensure_default_branch(repo: &Utf8Path) -> Result<()>` | Reattaches a detached clone to the branch named by `origin/HEAD`; repairs a missing `origin/HEAD` with `git remote set-head origin --auto`. | Call before `update_repository` when a previous pin may have detached the managed clone. It is a no-op when `HEAD` already names a branch and does not pull changes itself. | + +`CommitSha` is the Git adapter's validated full object-ID type. Keep commit +provenance in this type across Git and workspace boundaries; convert to text +only at output or external-format boundaries. + +#### Workspace API and provenance + +`workspace::ensure_workspace(dirs, update, git_ref)` returns a +`WorkspaceCheckout` containing the prepared `root`, the selected +`WorkspaceAction`, and commit provenance. `pinned_commit` is the resolved +`CommitSha` when `git_ref` pins the managed clone. `detached_commit` records the +existing detached `HEAD` reused by an unpinned `--no-update` install. These +fields are mutually exclusive for a single checkout. + +`WorkspaceCheckout::expected_git_sha() -> Option<&CommitSha>` returns the pinned +commit, or the inherited detached commit, as an optional full object ID. Pass +that value to the prebuilt path so a pinned or detached checkout can reuse an +artefact only when its manifest records the same full object ID. A rolling +install with no commit provenance leaves it unset. `ensure_ref_allowed` must +reject a pin when +the selected action is `UseCurrentDir`, because checking out a ref there would +mutate the user's working tree. + +`workspace::ensure_workspace` acquires the internal `ManagedCloneLock` while +preparing the installer-managed clone. The exclusive advisory sidecar is +stored beside the clone as `.lock` and covers action +selection, ref resolution, checkout, and update, so concurrent installers do +not race on shared Git state. Current-directory workspaces do not need this +lock; keep the lock scoped to workspace preparation and surface acquisition +failures as `InstallerError::WorkspaceLock`. + +The installer binary initializes one stderr tracing subscriber at startup. +`RUST_LOG` controls its `EnvFilter`; without it, the default level is `warn`. +Use, for example, `RUST_LOG=whitaker_installer=debug` when diagnosing Git or +workspace preparation. Library code must continue emitting events through +`tracing` without installing a global subscriber. + #### `resolve_additional_components` ```rust @@ -1893,10 +1986,11 @@ struct FastPathContext<'a> { requested_crates: &'a [CrateName], toolchain: &'a Toolchain, target_dir: &'a Utf8PathBuf, + expected_git_sha: Option<&'a CommitSha>, } ``` -A parameter-object struct that bundles the five immutable inputs consumed by +A parameter-object struct that bundles the six immutable inputs consumed by `try_fast_path_installation`. This follows the same idiom used elsewhere in the codebase (`FinishInstallContext`, `PrebuiltInstallationContext`, `MetricsWriteContext`) to keep function argument counts within the project diff --git a/docs/execplans/issue-271-ref-pinned-installation.md b/docs/execplans/issue-271-ref-pinned-installation.md new file mode 100644 index 00000000..d2e61628 --- /dev/null +++ b/docs/execplans/issue-271-ref-pinned-installation.md @@ -0,0 +1,845 @@ +# Add a `--ref` flag to whitaker-installer for pinned suite installation + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: IMPLEMENTED + +Issue: [leynos/whitaker#271](https://github.com/leynos/whitaker/issues/271) + +## Purpose / big picture + +`whitaker-installer` currently installs the lint suite from a mutable source: +the prebuilt path downloads from the `rolling` GitHub release tag, and the +source-build fallback clones or pulls the default branch of +`leynos/whitaker`. Consuming repositories pin the *installer* version in CI +(for example `cargo binstall whitaker-installer@0.2.5`) but cannot pin the +*suite*, so a push to `main` can change lint behaviour across the whole +estate at once. + +After this change, a user can run: + +```bash +whitaker-installer --ref v0.2.5 # a tag +whitaker-installer --ref 1a2b3c4d… # a commit SHA +``` + +and the installer will build and stage the lint suite from exactly that +commit of `leynos/whitaker`, carrying its resolved full object ID as +provenance. Running `whitaker-installer` with no `--ref` +behaves exactly as today (rolling prebuilt artefacts, default-branch source +fallback). Rolling remains the intentional default; the maintainer does not +want to cut a new installer release for every suite update. This plan +deliberately implements only the explicit-pin part of issue #271, not the +version-matched default proposed there. + +Observable success: `whitaker-installer --ref --build-only` on a clean +machine stages libraries built from the tagged commit (verifiable because the +staged filenames embed the toolchain channel recorded in that commit's +`rust-toolchain.toml`), `whitaker-installer --dry-run --ref ` reports the +ref, and a subsequent un-pinned `whitaker-installer` run still works (the +clone recovers from the detached checkout). A pinned prebuilt is accepted only +when its manifest records the same full object ID as the resolved ref. + +## Constraints + +- With no `--ref`, ordinary operation must continue to try the rolling prebuilt + download first, then use the default-branch clone/pull fallback. If a prior + pinned installation left the managed clone detached, the no-ref path may + reattach it to the default branch before performing the unpinned update. +- The public crate API additions must be additive; no existing public + function signatures in `whitaker_installer` may change in a way that breaks + the behaviour tests' existing imports, except where the plan names the + change explicitly (see `Interfaces and dependencies`). +- The installer must never mutate a user's own working tree: if the current + directory is itself a Whitaker workspace, `--ref` must fail with a clear + error rather than checking anything out. +- Runtime diagnostics use the `tracing-subscriber` dependency with its + `env-filter` feature. Test support may directly declare the already-resolved + `cap-std` crate solely for capability-scoped fixture writes. Git operations + continue to go through `installer/src/git.rs` with the existing 5-minute + timeout discipline. +- All work follows the repository gates: `make check-fmt`, `make lint`, + `make test`, `make markdownlint` must pass before each commit. +- Commit messages follow the file-based workflow (`git commit -F`), no AI + attribution trailers, en-GB-oxendict prose. +- This shell exports a stray `WHITAKER=true`; run every make gate with + `env -u WHITAKER` to avoid silently skipping the whitaker lint target. + +## Tolerances (exception triggers) + +- Scope: if implementation (excluding tests and docs) exceeds ~400 net lines + or touches more than 12 source files, stop and escalate. +- Interface: if pinning turns out to require changing the signature of a + public function other than those listed in `Interfaces and dependencies`, + stop and escalate. +- Dependencies: if an additional crate dependency appears necessary, stop and + escalate. +- Iterations: if a gate still fails after 3 fix attempts on the same failure, + stop and escalate. +- Ambiguity: if `--ref` semantics interact with an existing flag in a way not + settled by the Decision Log (for example a new conflict with + `--no-update`), stop and present options. + +## Risks + +- Risk: a detached-HEAD checkout left by `--ref` breaks the next un-pinned + install, because `ensure_workspace` runs plain `git pull`, which fails on a + detached HEAD ("You are not currently on a branch"). + Severity: high. Likelihood: certain without mitigation. + Mitigation: the update path must reattach the clone to the default branch + before pulling (Stage C step 3). This is a required behaviour, tested. +- Risk: a prebuilt manifest could claim a different commit from the resolved + pin. Severity: high. Likelihood: possible when rolling artefacts are reused. + Mitigation: `resolve_commit` and `fetch_ref` return a validated full + `CommitSha`; pinned installations require exact full-object-ID equality before + accepting a prebuilt. Rolling installs leave this expectation unset. +- Risk: tags in the whitaker repository may not exist for every released + installer version, making `--ref v0.2.5` fail for users. + Severity: low (documentation issue, not a code defect). + Mitigation: document that `--ref` accepts any commit-ish that exists in the + repository; error messages surface the git failure verbatim. +- Risk: behaviour tests that construct `InstallArgs` literally will fail to + compile when a field is added. + Severity: low. Likelihood: high. + Mitigation: most construction sites use `..InstallArgs::default()`; sweep + with `env -u WHITAKER cargo check --workspace --all-targets` early in + Stage C. + +## Progress + +- [x] (2026-07-08 12:20Z) Worktree created at + `~/Projects/whitaker.worktrees/issue-271-ref-pinned-installation`, branch + `issue-271-ref-pinned-installation` from `origin/main` (b1c69c2). +- [x] (2026-07-08 12:40Z) Read the installer source: `cli.rs`, `main.rs`, + `install_flow.rs`, `workspace.rs`, `git.rs`, `prebuilt.rs`, + `artefact/download.rs`; mapped the install flow and identified the + detached-HEAD recovery requirement. +- [x] (2026-07-08 12:50Z) Drafted this ExecPlan. +- [x] (2026-07-08) User approved the plan; proceeding through all stages. +- [x] (2026-07-08, superseded historical record) Stage A confirmed the + manifest `git_sha` was abbreviated (`git rev-parse --short HEAD`) and fixed + the then-current comparison rule; the full-object-ID contract below now + supersedes that rule. +- [x] (2026-07-08) Stage B/C slice 1: `--ref` CLI field. Red (E0609 no field + `git_ref`) → green (26 cli tests). Commit 6e36c2d. +- [x] (2026-07-08) Stage B/C slice 2: git helpers `resolve_commit`, + `fetch_ref`, `checkout_detached`, `ensure_default_branch`. Red (E0425 missing + functions) → green (8 real-git TempDir tests). See + . +- [x] (2026-07-08) Stage B/C slice 3: workspace plumbing — `WorkspaceCheckout`, + `ensure_workspace(dirs, update, git_ref)`, `RefUnsupported`, requested-ref + fetching with local fallback, and `ensure_default_branch` before every + update. Red (E0425/E0599) → green. + See + . +- [x] (2026-07-08, superseded historical record) Stage B/C slice 4 added + prefix-tolerant prebuilt `expected_git_sha` validation. The current + contract requires exact full-object-ID provenance. +- [x] (2026-07-08) Stage B/C slice 5: dry-run `git_ref` field, pinning progress + messages, 2 CLI BDD scenarios (pinned dry-run, refuse in workspace). Red + (missing "Pinned ref:" line) → green. Commit 4caa7d6. +- [x] (2026-07-08) Stage D: documented `--ref` in `docs/users-guide.md` + (new "Pinning the suite" subsection) and `README.md`; `--help` examples + refreshed in `cli.rs`. Full gates green. +- [x] (2026-07-08) Manual end-to-end validation transcript recorded under + `Artefacts`; platform clone restored to `main` by the recovery run. +- [x] (2026-08-01) Review follow-up replaced direct `InstallerError::Git` + construction checks with failures driven through `clone_repository` and + `update_repository`, and added a real-Git pin, detach, reattach, and update + lifecycle test. +- [x] (2026-08-01, superseded historical record) Added two proptest properties + for accepted 7–40 character SHA prefixes and rejection after changing a + prefix nibble. +- [x] (2026-08-01) Full review gates passed: formatting, tests, type-checking, + linting, Markdown, and Mermaid validation. +- [x] (2026-08-01, superseded historical record) Verified the review findings + and rejected the full-SHA-only proposal because rolling producers recorded + abbreviated SHAs. The current implementation has since adopted exact + full-object-ID validation. +- [x] (2026-08-01) Converted `git_fixture` to an injected `rstest` fixture; + added real-Git coverage for missing `origin/HEAD` repair and fetched pinning; + extracted `finalize_workspace_checkout`; routed prebuilt test writes through + a directory capability; and applied the documentation corrections. +- [x] (2026-08-01) Historical review validation passed: `make check-fmt` + (`/tmp/final-current-check-fmt-7b3c543c-issue-271-ref-pinned-installation.out`); + `make test`, 1,482/1,482 passed and 3 skipped + (`/tmp/final-current-test-7b3c543c-issue-271-ref-pinned-installation.out`); + `make typecheck` + (`/tmp/final-current-typecheck-7b3c543c-issue-271-ref-pinned-installation.out`); + and `make lint` (`cargo doc` + Clippy) + (`/tmp/final-current-lint-7b3c543c-issue-271-ref-pinned-installation.out`). + Markdownlint reported 0 errors + (`/tmp/final-current-markdownlint-7b3c543c-issue-271-ref-pinned-installation.out`), + and Nixie validated all diagrams + (`/tmp/final-current-nixie-7b3c543c-issue-271-ref-pinned-installation.out`). +- [x] (2026-08-04) Rebased onto `origin/main` and verified the latest review + findings. The implementation now fetches a requested ref into a dedicated + local ref before resolving it, falling back to local resolution only when + fetch fails; `WorkspaceCheckout` carries the selected `WorkspaceAction` so + progress reporting uses the action that was actually performed. +- [x] (2026-08-04) Final post-rebase validation passed: `make check-fmt` + (`/tmp/issue271-rebase-final-check-fmt.log`); `make test`, 1,688/1,688 + passed, 5 skipped, and 7 slow (`/tmp/issue271-rebase-final-test.log`); + `make typecheck` (`/tmp/issue271-rebase-final-typecheck.log`); and + `make lint`, including Cargo documentation and Clippy + (`/tmp/issue271-rebase-final-lint.log`). Markdownlint initially found one + extra blank line; after the Scribe correction it passed 76 files with 0 + errors (`/tmp/issue271-rebase-docfix-markdownlint.log`). Nixie passed 76 + files and 15 Mermaid diagrams (`/tmp/issue271-rebase-docfix-nixie.log`). +- [x] (2026-08-22) Current interface and provenance follow-up: Git commit + resolution returns `CommitSha` full object IDs; workspace and prebuilt + interfaces carry and compare that provenance exactly. Current tests cover + clone/update, ref resolution and fetching, detached checkout and recovery, + workspace provenance, and exact prebuilt match/mismatch behaviour. +- [x] (2026-08-22) Managed-clone preparation is serialized by + `ManagedCloneLock` from action selection through checkout. The lock creates + its parent directory and sidecar through `cap_std::fs_utf8::Dir`, converting + the capability-bound file with `into_std()` only immediately before + `fs2::FileExt::lock_exclusive()`; waiters then re-evaluate workspace state. + +## Surprises & discoveries + +- (2026-07-08, Stage A, superseded historical record) The manifest `git_sha` + was **abbreviated**, not a full 40-hex SHA, and the then-current comparison + rule was prefix-tolerant. The current pinned-install contract requires a + manifest full object ID to equal the resolved `CommitSha` exactly. +- (2026-07-08, Stage A) Default-branch discovery: the platform clone carries + `refs/remotes/origin/HEAD` symbolic ref (`git symbolic-ref + refs/remotes/origin/HEAD` → `refs/remotes/origin/main`; `git rev-parse + --abbrev-ref origin/HEAD` → `origin/main`). `ensure_default_branch` uses + `git rev-parse --abbrev-ref origin/HEAD`, strips the `origin/` prefix to get + the branch name, and falls back to `git remote set-head origin --auto` once + when the symbolic ref is absent from an older clone. The repository default + branch is `main`. +- (2026-07-08, Stage A, superseded historical record) The shared test manifest + helper `test_utils::prebuilt_manifest_json` hardcodes `"git_sha":"abc1234"`. + The + prebuilt SHA-match tests therefore keyed off that literal and a shared + prefix. Current tests use full-object-ID match and mismatch cases. +- (2026-08-04, review pass) Resolving `FETCH_HEAD` after a fetch is ambiguous + when Git records more than one fetched object. Fetching the requested + refspec into `refs/whitaker/pinned-ref` gives pinning a stable name to resolve + regardless of unrelated tags. + +## Decision log + +- Decision: keep `rolling` as the default; `--ref` is an explicit opt-in pin. + Rationale: maintainer direction — a version-matched default would force an + installer release for every suite update. This supersedes the "default to + the installer's own version tag" proposal in issue #271. + Date/Author: 2026-07-08, user (pmcintosh). +- Decision: name the flag `--ref` (CLI field `git_ref`, since `ref` is a Rust + keyword), accepting any commit-ish (SHA, tag, or branch name), documented + with SHA and tag as the supported use cases. + Rationale: user asked for "--ref … allowing a SHA or tag"; git resolves all + commit-ish forms identically, so restricting the value would add validation + code without adding safety. Branch names resolve at install time and are + therefore not reproducible pins; the documentation says so. + Date/Author: 2026-07-08, agent. +- Decision: `--ref` composes with `--no-update` rather than conflicting. + With both flags, the installer first attempts to fetch the requested ref into + its dedicated local ref. If fetching fails, it resolves an already-available + local ref or SHA, preserving offline pinned installations. + Rationale: "use my existing clone, offline, at this ref" is a coherent and + useful request; fetch-first avoids silently pinning a stale local branch, + while local fallback preserves the offline case. + Date/Author: 2026-08-04, review feedback. +- Decision: with `--ref`, the prebuilt fast path is attempted only when the + resolved commit SHA matches the rolling manifest's `git_sha`; any mismatch + falls back to a source build of the pinned commit. Without `--ref`, + prebuilt behaviour is unchanged. + Rationale: the rolling release only ever carries artefacts for one commit, + so a pinned install can use it only when the pin happens to match; the + existing `PrebuiltResult::Fallback` mechanism makes the mismatch case free. + Date/Author: 2026-07-08, agent. +- Decision: `--ref` while the current directory is itself a Whitaker + workspace is an error (`InstallerError::WorkspaceNotFound` is wrong here; a + new `InstallerError::RefUnsupported`-style variant carries the message). + Rationale: checking out a ref in the user's own working tree could destroy + uncommitted work; refusing is the only safe behaviour. + Date/Author: 2026-07-08, agent. +- Decision (implementation, superseded historical record): the prebuilt SHA + match was *prefix-tolerant in one direction* because the manifest stored an + abbreviated SHA. The current decision requires exact full-object-ID + equality. + Date/Author: 2026-07-08, agent. +- Decision (process): BDD scenarios were landed with their related + implementation slice (prebuilt-mismatch with slice 4; pinned dry-run and the + in-workspace refusal with slice 5) rather than as a separate Stage B pass, so + each commit stays self-consistent and green. Every new test still failed first + for the intended reason (recorded in Progress and Artefacts). + Date/Author: 2026-07-08, agent. +- Decision (implementation): the user sees two messages when pinning — a + `Pinning Whitaker suite to REF...` progress line followed by a `Pinned + Whitaker suite to REF (SHORT_SHA).` confirmation. Workspace preparation + resolves the pin before reporting; the messages remain separate to preserve + the established progress output while confirming the resolved short SHA. + Date/Author: 2026-08-04, review feedback. +- Decision (architecture): `installer/src/git.rs` owns + `WHITAKER_REPO_URL`, because it is the only module that uses the value to + perform repository operations. `installer/src/workspace.rs` re-exports the + constant only to preserve its existing public path; new internal callers + import it from `git.rs`. + Rationale: this keeps the dependency direction one-way: workspace + orchestration calls git operations, while the git module does not depend on + workspace orchestration. + Date/Author: 2026-08-01, agent. +- Decision (verification, superseded historical record): use proptest for the + pure SHA-prefix invariant and + real Git repositories for detached-HEAD recovery. Do not add Kani or Verus + coverage for Git subprocess state. + Rationale: Kani cannot directly model the external Git process and + filesystem lifecycle, so a harness would test a stand-in rather than the + shipped implementation. No mathematical lemma is claimed, so Verus is not + applicable. The real-Git lifecycle test exercises the production functions. + Date/Author: 2026-08-01, agent. +- Decision (review scope): `workspace::finalize_workspace_checkout` is called + only from `ensure_workspace` action arms after their setup operations. It may + combine optional pinning with `WorkspaceCheckout` construction only. Clone, + update, and detached-head reattachment operations remain in + `ensure_workspace` so its match arms continue to make repository state + transitions explicit. + Rationale: this removes repeated result construction without hiding the + orchestration that distinguishes the workspace setup paths. + Date/Author: 2026-08-01, review feedback. +- Decision (review provenance, superseded historical record): retain + prefix-tolerant pinned-prebuilt validation and its existing proptest + properties; do not require a full manifest object ID in that review pass. + Rationale: both rolling-release producers recorded abbreviated SHAs. + Date/Author: 2026-08-01, agent. +- Decision (current provenance): represent resolved Git commits as `CommitSha` + full object IDs. A pinned or inherited detached checkout may reuse a + prebuilt only when the manifest's full object ID equals the expected value; + rolling installs leave the expected value unset. + Rationale: a short or shared-prefix SHA cannot prove that the artefact was + built from the selected commit. + Date/Author: 2026-08-22, agent. +- Decision (review tests): inject the shared real-Git repository through an + `rstest` fixture, exercise missing-default-ref recovery and remote-only + pinning through production flows, and use a `cap_std::fs::Dir` rooted at each + test file's parent for mock writes. + Rationale: these changes test Git's actual ref behaviour, remove repeated + fixture setup, and keep filesystem authority scoped to temporary test data. + Date/Author: 2026-08-01, agent. +- Decision (review progress): `ensure_workspace` returns the selected + `WorkspaceAction` in `WorkspaceCheckout`; the reporter renders that recorded + action after setup rather than recomputing it from the current directory and + clone path. Pinning retains its two messages: `Pinning Whitaker suite to + REF...` and, after resolution, `Pinned Whitaker suite to REF (SHORT_SHA).` + Rationale: reporting the executed action prevents filesystem state changes + from making the displayed action diverge from the action that was selected. + Date/Author: 2026-08-04, review feedback. + +## Outcomes & retrospective + +Delivered across six commits on `issue-271-ref-pinned-installation`: the CLI +`--ref` field, four git helpers, workspace pinning plumbing +(`WorkspaceCheckout` + `ensure_workspace(dirs, update, git_ref)`), prebuilt +`expected_git_sha` gating, dry-run/progress messaging, and documentation. All +gates (`check-fmt`, `lint`, `test`, `markdownlint`) passed before each commit; +the historical initial-implementation `make test` snapshot ran 1,472 tests +(1,472 passed, 3 skipped). Every new test failed first for the intended reason +(see `Artefacts`). The manual smoke test +exercised the pin, the in-workspace refusal, the toolchain-embedded staged +filename, and the detached-HEAD recovery end-to-end against tag `v0.2.4`. + +Scope stayed within tolerances: the implementation touched the CLI, git, +workspace, prebuilt, install-flow, output, error, and main modules (well under +the 12-file limit), added the `tracing-subscriber` runtime dependency, and made +only the one sanctioned public signature change (`ensure_workspace`). Rolling +remains the default; only the explicit-pin part of issue #271 is implemented, +and the version-matched default proposed there was deliberately not built +(Decision Log). + +Superseded historical lesson: confirming the manifest SHA width in Stage A was +load-bearing — the match was prefix-tolerant, and exact equality would have +silently disabled the prebuilt fast path for every pin. (2) Adding a field to +`PrebuiltConfig` and the +context structs surfaced literal constructors in the binary's own test modules +(`install_flow/tests.rs`, `tests/fast_path.rs`) that `cargo test --lib` did not +compile; an early `cargo check --all-targets` sweep catches these. + +Superseded historical review record: the latest review follow-up then +contained the minimal valid changes: reusable +`rstest` Git setup, real-Git regressions for default-branch repair and fetched +pinning, capability-scoped prebuilt fixture writes, a private +checkout finalizer with an explicit orchestration boundary, and terminology +corrections. The test-only `cap-std` declaration makes an already-resolved +crate available directly to the installer tests; no runtime dependency was +added. The requested full-SHA-only comparison was deliberately unimplemented +at that point because it conflicted with the rolling manifest format. A +historical review snapshot reported 1,482/1,482 tests passed with 3 skipped; +formatting, type-checking, Cargo documentation, Clippy, Markdownlint, and Nixie +were clean. + +Superseded historical review record (2026-08-01): the follow-up strengthened +the real-Git failure and lifecycle +coverage, adds SHA-prefix property tests, and removes the workspace-to-git +module cycle. `make check-fmt`, `make typecheck`, and `make lint` passed; lint +included rustdoc and Clippy with warnings denied. That historical pre-rebase +`make test` result was 1,481 tests passed, with 3 skipped and 1 slow. +`make markdownlint` checked 70 files with 0 errors, and `make nixie` reported +all diagrams valid. + +Superseded historical review record (2026-08-04): the review pass made pinning +fetch the requested ref into a +dedicated local ref before resolving it, with local resolution as the offline +fallback. It also records the selected `WorkspaceAction` in +`WorkspaceCheckout`, so progress output describes the operation that +`ensure_workspace` actually selected. The exact-full-SHA proposal was then +rejected because current rolling producers emitted abbreviated SHAs. The +authoritative final `make test` result is 1,688/1,688 passed, with 5 skipped +and 7 slow. Formatting, type-checking, Cargo documentation, Clippy, +Markdownlint, and Nixie also passed; the final documentation checks covered 76 +files and 15 Mermaid diagrams. + +Current contract: `resolve_commit` and `fetch_ref` return `CommitSha`, a +validated full object ID. `WorkspaceCheckout` carries `pinned_commit` or an +inherited `detached_commit`, and `expected_git_sha()` supplies that provenance +to exact prebuilt validation. Current coverage exercises all public Git +operations, detached-checkout recovery, workspace provenance, and exact +prebuilt match and mismatch outcomes. + +## Context and orientation + +The repository is a Cargo workspace (`Cargo.toml` members: `common`, +`crates/*`, `installer`, `suite`). The installer lives in `installer/` and +publishes to crates.io as `whitaker-installer`. Key modules, all paths +relative to the repository root: + +- `installer/src/cli.rs` — clap definitions. `InstallArgs` is the flag + struct; it has a handwritten `Default` impl and is flattened into `Cli` + for the default (no-subcommand) install. Tests in + `installer/src/cli_tests.rs`. +- `installer/src/main.rs` — orchestration. `run_install` performs: (1) + dependency check, (2) `ensure_whitaker_workspace` (clone or update the + platform clone at `~/.local/share/whitaker` on Linux), (3) crate and + toolchain resolution (reads `rust-toolchain.toml` from the workspace root — + note this happens *after* the workspace step, so a checked-out ref + naturally supplies its own toolchain pin), (3.5) + `try_fast_path_installation` (prebuilt download, then a test-only staged + suite path), (4) source build via `pipeline::perform_build`, (5) wrapper + scripts and metrics. +- `installer/src/workspace.rs` — the compatibility re-export of + `WHITAKER_REPO_URL`, `WorkspaceAction` (`UseCurrentDir` / `CloneTo` / + `UpdateAt` / `UseExisting`), + `decide_workspace_action(cwd, clone_dir, update)`, and + `resolve_workspace_action(dirs, update)`. The latter selects from the live + environment, while `ensure_workspace(dirs, update, git_ref)` executes that + action and returns it in `WorkspaceCheckout` with the root, selected action, + and optional pinned or inherited detached commit provenance. +- `installer/src/git.rs` — the owning definition of `WHITAKER_REPO_URL`, the + public clone, update, ref-resolution, fetch, detached-checkout, detached + HEAD, and default-branch operations, and the private + `run_git_with_timeout(args, working_dir, operation)` helper (5-min timeout, + threaded pipe draining). All new Git operations reuse this helper. Commit + resolution returns the full-object-ID `CommitSha` type. +- `installer/src/prebuilt.rs` — `PrebuiltConfig` (target, toolchain, + destination, quiet), `attempt_prebuilt`, and the private `run_pipeline` + that downloads the manifest, validates toolchain and target, downloads and + verifies the archive, and extracts. Validation failures become + `PrebuiltResult::Fallback` — never fatal. +- `installer/src/artefact/download.rs` — `ROLLING_TAG = "rolling"` and URL + construction; `installer/src/artefact/manifest.rs` and + `artefact/git_sha.rs` — the manifest carries a `git_sha()` accessor. Rolling + manifests may remain abbreviated; pinned prebuilt reuse requires a full + object ID matching the expected `CommitSha` exactly. +- `installer/src/install_flow.rs` — `PrebuiltInstallationContext` and + `try_prebuilt_installation`, the seam through which `main.rs` passes data + into `prebuilt.rs`. +- `installer/src/output.rs` — `DryRunInfo` renders `--dry-run` output. +- Behaviour tests: `installer/tests/behaviour_*.rs` with Gherkin features in + `installer/tests/features/*.feature`, driven by `rstest-bdd`. CLI-facing + scenarios live in `installer.feature`; prebuilt scenarios in + `prebuilt_download.feature`. +- Docs: `docs/users-guide.md` (user-facing flags), `README.md`. + +Terms: a *commit-ish* is any expression git can resolve to a commit (SHA, +tag, branch, `HEAD~2`, …). A *detached HEAD* is a checkout of a commit rather +than a branch; `git pull` refuses to run on one. The *platform clone* is the +installer-managed copy of this repository under the user's data directory. + +## Plan of work + +Stage A (superseded historical plan) — confirm two facts, no code changes. +First, read +`installer/src/artefact/git_sha.rs`, `artefact/manifest.rs`, and the release +workflow under `.github/workflows/` to establish whether the manifest +`git_sha` is a full 40-hex SHA or abbreviated; record the answer in +`Surprises & Discoveries` and fix the then-current comparison rule (exact match +for full, `starts_with` for abbreviated). The current rule is exact equality +between full object IDs. Second, decide the default-branch discovery +command for detached-HEAD recovery: prefer +`git rev-parse --abbrev-ref origin/HEAD` (strips to `origin/main`), falling +back to `git remote set-head origin --auto` once if the symbolic ref is +absent from an old clone. + +Stage B — red tests. Add failing tests before each implementation slice: + +1. `installer/src/cli_tests.rs`: parsing `--ref v0.2.5` populates + `InstallArgs::git_ref`; absence leaves it `None`; `--ref` is accepted both + bare and under the `install` subcommand. +2. `installer/src/git_tests.rs` tests use real `git` against `TempDir` + fixtures. They cover tag, branch, and SHA resolution; invalid refs; detached + checkout; tag and remote-branch fetches; branch no-op behaviour; real clone + and pull failures; and the full pin, detach, remote advance, reattach, and + update lifecycle. +3. `installer/src/workspace.rs` tests: `decide_workspace_action` outcomes + are unchanged; a new decision-level test that a ref plus + `UseCurrentDir` yields the refusal error. +4. `installer/src/prebuilt_tests.rs`: with `expected_git_sha: Some(...)` + mismatching the mocked manifest, the pipeline returns + `PrebuiltResult::Fallback` whose reason mentions the SHA mismatch; with a + matching SHA it succeeds as before; with `None` behaviour is unchanged. +5. BDD: extend `installer/tests/features/installer.feature` (and + `behaviour_cli` steps) with scenarios: "Pin the suite to a tag" (parsing + plus dry-run output contains the ref) and "Refuse --ref inside a Whitaker + workspace". Extend `prebuilt_download.feature` with "Prebuilt is skipped + when the pinned ref does not match the rolling manifest". + +Each red test is run first and must fail for the expected reason (missing +field / missing function / missing validation) before its green slice lands. + +Stage C — implementation, in five small commits: + +1. CLI: add `git_ref: Option` to `InstallArgs` in + `installer/src/cli.rs` as `#[arg(long = "ref", value_name = "REF")]`, doc + comment "Install the lint suite at a specific commit SHA or tag [default: + rolling]". Update `Default`, the `after_help` examples, and any literal + constructors that fail to compile. +2. Git helpers in `installer/src/git.rs`: `resolve_commit(repo, refspec) -> + Result` (`git rev-parse --verify ^{commit}`), + `fetch_ref(repo, refspec) -> Result` + (fetch the requested refspec into `refs/whitaker/pinned-ref`, then resolve + that dedicated ref), + `checkout_detached(repo, commit: &CommitSha)` (`git checkout --detach + `), `detached_head_commit(repo) -> Result>`, and + `ensure_default_branch(repo)` (no-op when already on a branch; otherwise + discover the default branch per Stage A and check it out). All through + `run_git_with_timeout`. +3. Workspace plumbing: extend `ensure_workspace` to + `ensure_workspace(dirs, update, git_ref: Option<&str>)` (public signature + change, named in `Interfaces and dependencies`). Behaviour: on + `UseCurrentDir` with a ref → the refusal error; on `CloneTo` → clone then + pin; on `UpdateAt` → `ensure_default_branch` then pull then pin; on + `UseExisting` with a ref → pin without pulling. "Pin" means: try + `fetch_ref` first; if fetching fails, try `resolve_commit` locally; then + `checkout_detached`. Crucially, even an unpinned + `UpdateAt` calls `ensure_default_branch` first, fixing the recovery risk. + Return the selected action, workspace path, and resolved full-object-ID + `CommitSha` in `WorkspaceCheckout` so `main.rs` can hand provenance to the + prebuilt path and render progress from the action that was actually + selected. Preserve an inherited detached `CommitSha` for an unpinned + `--no-update` checkout. +4. Prebuilt: add `expected_git_sha: Option<&'a CommitSha>` to `PrebuiltConfig`, + validate in `run_pipeline` after the target check using exact full-object-ID + equality, and thread the value from `run_install` through + `PrebuiltInstallationContext` and `try_prebuilt_installation`. Update the + fixed-signature `AttemptPrebuiltFn` type alias and hooks in + `install_flow.rs` as needed. +5. Dry run and messaging: add the ref to `DryRunInfo` and its `display_text`. + After workspace setup, pinning progress prints `Pinning Whitaker suite to + REF...`, followed by `Pinned Whitaker suite to REF (SHORT_SHA).` + Workspace-action progress is rendered from `WorkspaceCheckout.action` + rather than by repeating the action-selection inputs. + +Stage D — documentation and closure: document `--ref` in +`docs/users-guide.md` (a "Pinning the suite" subsection: what it accepts, +that branch pins are not reproducible, prebuilt-match behaviour, interplay +with `--no-update`) and in `README.md`; refresh the `--help` examples; run +the full gate suite via the scrutineer subagent; update this plan's living +sections; comment on issue #271 describing what shipped and what was +deliberately not changed (the rolling default). + +## Concrete steps + +All commands run from +`~/Projects/whitaker.worktrees/issue-271-ref-pinned-installation`. + +Red example (Stage B slice 1): + +```bash +env -u WHITAKER cargo nextest run -p whitaker-installer cli_tests 2>&1 \ + | tee /tmp/test-whitaker-issue-271.out +# expect: compile error E0609 (no field `git_ref`) or a failing +# `parses_ref_flag` assertion — the red reason must be the missing field. +``` + +Green example (after Stage C slice 1): + +```bash +env -u WHITAKER cargo nextest run -p whitaker-installer cli_tests 2>&1 \ + | tee /tmp/test-whitaker-issue-271.out +# expect: all cli tests pass. +``` + +Full gates before each commit (delegate to the scrutineer subagent, which +logs to /tmp and returns a bounded report): + +```bash +env -u WHITAKER make check-fmt +env -u WHITAKER make lint +env -u WHITAKER make test +env -u WHITAKER make markdownlint +``` + +Manual end-to-end check (Stage D, uses an older tag known to exist): + +```bash +git tag --list 'v*' | tail -3 # pick a real tag, e.g. v0.2.4 +cargo run -p whitaker-installer -- --dry-run --ref v0.2.4 +# expect: dry-run output includes the ref and the workspace path +cargo run -p whitaker-installer -- --ref v0.2.4 --build-only \ + --target-dir /tmp/whitaker-ref-smoke +# expect: staged libraries under /tmp/whitaker-ref-smoke named for the +# toolchain channel pinned by v0.2.4's rust-toolchain.toml +cargo run -p whitaker-installer -- --dry-run +# expect: default behaviour unchanged; then verify the platform clone +# recovers: run a default install and confirm no detached-HEAD pull error +``` + +## Validation and acceptance + +Acceptance is behavioural: + +1. `whitaker-installer --ref --build-only` stages libraries built from + the tagged commit; `whitaker-installer --dry-run --ref ` prints the + ref. +2. `whitaker-installer --ref ` exits non-zero with a git error + naming the ref; nothing is staged. +3. `whitaker-installer --ref ` run inside a Whitaker workspace + checkout exits non-zero with the refusal message and does not touch the + working tree. +4. After a pinned install, a plain `whitaker-installer` run succeeds (the + clone reattaches to the default branch and pulls). +5. With `--ref` set and the rolling manifest's full object ID differing from + the resolved `CommitSha`, the installer prints the fallback message and + builds from source; when they match exactly, the prebuilt path is used. +6. `make test` passes with the new unit and behaviour tests; each new test + demonstrably failed first (Red evidence retained in the tee'd logs under + `/tmp/test-whitaker-issue-271.out` and summarized in `Artefacts`). +7. Commit-resolution and prebuilt tests preserve full-object-ID provenance: + matching full IDs are accepted and abbreviated or shared-prefix IDs are + rejected for pinned installations. +8. `make check-fmt`, `make lint` (clippy plus the dylint suite), and + `make markdownlint` pass. + +BDD specification driving slice 5 of Stage B (final wording may be adjusted +to the step vocabulary in `installer/tests/behaviour_cli/`): + +```gherkin +Scenario: Pin the suite to a tag + Given the --ref flag is set to "v0.2.5" + When the CLI arguments are parsed + Then the install arguments carry the ref "v0.2.5" + +Scenario: Refuse --ref inside a Whitaker workspace + Given the current directory is a Whitaker workspace + And the --ref flag is set to "v0.2.5" + When the workspace is prepared + Then preparation fails with a ref-unsupported error + +Scenario: Prebuilt is skipped when the pinned ref does not match + Given a manifest whose git SHA differs from the pinned commit + When a prebuilt installation is attempted + Then the result is a fallback mentioning the SHA mismatch +``` + +## Idempotence and recovery + +All steps are re-runnable. The worktree is disposable; `git worktree remove` +plus branch deletion resets everything. Test git repositories live in +`TempDir`s and clean themselves up. The manual smoke test writes only to +`/tmp/whitaker-ref-smoke` (delete freely) and the platform clone at +`~/.local/share/whitaker`, which the recovery behaviour (acceptance item 4) +restores to the default branch; if a mid-implementation failure leaves it +detached, `git -C ~/.local/share/whitaker checkout main` restores it by +hand. If a gate fails mid-commit, fix and re-run the gate; never commit over +a red gate. + +## Artefacts and notes + +Red/Green evidence (gate logs under `/tmp/*-whitaker-issue-271*.out`): + +- Slice 1 (CLI): red — `E0609: no field git_ref on InstallArgs`; green 26 cli + tests pass. +- Slice 2 (git helpers): red — `E0425: cannot find function resolve_commit` + (and the other three helpers); green 8 tests pass against real git TempDir + repos. +- Slice 3 (workspace): red — `E0425: cannot find function ensure_ref_allowed` + plus `E0599: no variant named RefUnsupported`; green 20 workspace tests pass. +- Slice 4 (prebuilt): red — `expected_git_sha_mismatch_returns_fallback` got + `Success` when validation was absent; green 3 unit tests + the + `Prebuilt is skipped when the pinned ref does not match` BDD scenario pass. +- Slice 5 (dry-run/messaging): red — `dry_run_display_includes_ref_when_pinned` + failed (no `Pinned ref:` line); green 2 output tests + the pinned dry-run and + in-workspace refusal CLI BDD scenarios pass. +- Full suite green each slice: the historical initial-implementation + `make test` snapshot reported 1,472 tests run, 1,472 passed, and 3 skipped. + +Superseded historical review follow-up test inventory (full gates passed): + +- `installer/src/git_tests.rs` has nine real-Git tests. The lifecycle test pins + through `workspace::pin_to_ref`, confirms detached HEAD, advances the remote, + reattaches through `ensure_default_branch`, pulls through + `update_repository`, and confirms the new commit on `main`. +- The clone and update error tests invoke `clone_repository` and + `update_repository` against failing repositories before inspecting their + semantic `InstallerError::Git` fields. +- `installer/src/prebuilt_tests.rs` then had two proptest properties covering + valid manifest prefixes and a mismatched prefix across the supported SHA + lengths. + +Current test inventory covers every public Git operation: resolution across +tags, branches, and SHAs; fetching; detached checkout; default-branch repair; +the pin-and-recover lifecycle; and clone/update errors. Workspace tests cover +action selection and ref guarding. Prebuilt tests cover exact full-object-ID +matches, abbreviated IDs, and shared-prefix mismatches. +The managed-clone workflow test +`pinned_and_unpinned_preparations_serialize_and_recheck_state` holds the lock +during a pinned clone, confirms an unpinned preparation performs no repository +operations while waiting, then verifies it re-evaluates state, reattaches the +default branch, and updates after the lock is released. + +Manual smoke test (Stage D), run against tag `v0.2.4` +(commit `8512ee63a212abd175c31501a57e10a713881873`): + +- `whitaker-installer --dry-run --ref v0.2.4` (from the worktree) printed + `Pinned ref: v0.2.4` alongside the workspace root and toolchain. +- `whitaker-installer --ref v0.2.4 --skip-deps` (from the worktree, itself a + Whitaker workspace) exited 1 with `cannot pin --ref v0.2.4: the current + directory is itself a Whitaker workspace...`. +- `whitaker-installer --ref v0.2.4 --build-only --skip-deps --skip-wrapper + --target-dir /tmp/whitaker-ref-smoke` (run from `/tmp/smoke-cwd`, outside any + workspace) updated then pinned the platform clone to a detached HEAD at + `8512ee63a212`, built the suite under that commit's `nightly-2025-09-18` + toolchain, and staged `libwhitaker_suite@nightly-2025-09-18.so` (filename + embeds the pinned toolchain channel). Exit 0. +- Recovery: a subsequent un-pinned `whitaker-installer --skip-deps + --skip-wrapper` reattached the detached clone to `refs/heads/main` before + pulling (no detached-HEAD error), then used the prebuilt fast path. Exit 0. + The platform clone at `~/.local/share/whitaker` is back on `main`. + +## Interfaces and dependencies + +The installer adds `tracing-subscriber` with its `env-filter` feature as a +runtime dependency for CLI diagnostics. It also adds the workspace's existing +`proptest` package as a development dependency. At completion the following +must exist: + +In `installer/src/cli.rs`: + +```rust +pub struct InstallArgs { + // …existing fields… + /// Install the lint suite at a specific commit SHA or tag. + #[arg(long = "ref", value_name = "REF")] + pub git_ref: Option, +} +``` + +In `installer/src/git.rs` (all `pub`, all routed through +`run_git_with_timeout`): + +```rust +pub const WHITAKER_REPO_URL: &str = "https://github.com/leynos/whitaker"; +pub fn clone_repository(target: &Utf8Path) -> Result<()>; +pub fn update_repository(repo: &Utf8Path) -> Result<()>; +pub fn resolve_commit(repo: &Utf8Path, refspec: &str) -> Result; +pub fn fetch_ref(repo: &Utf8Path, refspec: &str) -> Result; +pub fn checkout_detached(repo: &Utf8Path, commit: &CommitSha) -> Result<()>; +pub fn detached_head_commit(repo: &Utf8Path) -> Result>; +pub fn ensure_default_branch(repo: &Utf8Path) -> Result<()>; +``` + +`installer/src/workspace.rs` publicly re-exports `WHITAKER_REPO_URL` for +compatibility. The git module remains the owner, and new internal callers must +use `crate::git::WHITAKER_REPO_URL`. + +In `installer/src/workspace.rs` (signature change, the one sanctioned +breaking change; all in-repo callers updated in the same commit): + +```rust +pub struct WorkspaceCheckout { + pub root: Utf8PathBuf, + pub pinned_commit: Option, + pub detached_commit: Option, + pub action: WorkspaceAction, +} + +pub fn resolve_workspace_action( + dirs: &dyn BaseDirs, + update: bool, +) -> Result; + +pub fn ensure_workspace( + dirs: &dyn BaseDirs, + update: bool, + git_ref: Option<&str>, +) -> Result; +``` + +`WorkspaceCheckout::expected_git_sha()` returns the newly pinned commit, or an +inherited detached commit when no new `--ref` is supplied (including an +unpinned `--no-update` run), so the prebuilt SHA gate remains enabled for that +managed checkout. It is unset only for an ordinary rolling checkout. + +In `installer/src/prebuilt.rs`: + +```rust +pub struct PrebuiltConfig<'a> { + // …existing fields… + /// When set, require the manifest git SHA to match this full object ID. + pub expected_git_sha: Option<&'a CommitSha>, +} +``` + +`installer/src/install_flow.rs` passes +`WorkspaceCheckout::expected_git_sha()`, or equivalent expected commit +provenance, through `PrebuiltInstallationContext` into `PrebuiltConfig`. +`installer/src/output.rs`'s `DryRunInfo` gains a +`git_ref: Option<&'a str>` field rendered in `display_text`. + +## Revision note (2026-08-01, superseded historical record) + +Review follow-up clarified that `installer/src/git.rs` owns the repository URL +and `installer/src/workspace.rs` keeps only a compatibility re-export. It also +records the live real-Git lifecycle and failure tests, the two SHA-prefix +properties, the Kani and Verus scope decision, and `fetch_ref`'s +commit-returning signature. Full-gate validation passed with the results +recorded in `Outcomes & retrospective`. + +## Revision note (2026-08-01, latest review pass, superseded historical record) + +This revision uses “artefacts” consistently and clarifies that ordinary no-ref +behaviour remains rolling while allowing recovery from a detached clone left +by a prior pin. It records the verified finding dispositions, the implemented +test and helper changes, and the reason full-SHA-only validation was rejected +at that time. +That historical review validation included 1,482/1,482 tests with 3 skipped, +plus formatting, type-checking, Cargo documentation, Clippy, Markdownlint, and +Nixie. + +## Revision note (2026-08-04, rebase review pass, superseded historical record) + +This revision marks the abbreviated-SHA risk resolved by Stage A, labels the +1,472, 1,482, and 1,481 test snapshots as historical, and records the +authoritative final result of 1,688/1,688 passed, with 5 skipped and 7 slow. It +synchronizes the plan with fetch-first pinning through a dedicated local ref +and local fallback on fetch failure, action-carrying `WorkspaceCheckout`, and +progress rendered from the selected action. It also records the implemented +two-message pinning output, reaffirmed that exact full-SHA validation was +incompatible with abbreviated rolling producers, and records the clean final +formatting, type-checking, lint, Markdownlint, and Nixie results. + +## Revision note (2026-08-22) + +The current provenance contract uses the public `git::CommitSha` type for +full 40-hex object IDs returned by `resolve_commit` and `fetch_ref`. Git's +clone, update, checkout, detached-HEAD, and default-branch APIs are all +documented, and `WorkspaceCheckout` preserves pinned or inherited detached +provenance for exact prebuilt validation. Historical prefix-tolerant +comparison decisions remain above, explicitly labelled as superseded. diff --git a/docs/migrations/0.3.0.md b/docs/migrations/0.3.0.md new file mode 100644 index 00000000..8c3f6b4d --- /dev/null +++ b/docs/migrations/0.3.0.md @@ -0,0 +1,63 @@ +# Migrating to Whitaker 0.3.0 + +This guide covers the standalone installer changes introduced for the 0.3.0 +release. The existing Cargo workspace metadata integration continues to work; +the migration matters when the installer is responsible for selecting the +Whitaker suite revision. + +## What changes + +The installer now accepts `--ref ` to select the suite revision that it +builds and stages: + +```sh +whitaker-installer --ref +``` + +`REF` may be a tag, a full or abbreviated commit SHA, or a branch name. Use a +release tag or commit SHA for reproducible installations. A branch name is +resolved at install time and therefore moves when the branch moves. The +installer version and the suite revision are separate: installing +`cargo binstall whitaker-installer@0.3.0` selects the installer version, while +`--ref` selects the Whitaker commit it builds; see the +[Pinning the suite](../users-guide.md#pinning-the-suite) section for the +complete option and prebuilt-artefact reference. + +Without `--ref`, the installer continues to use the moving default branch and +the rolling prebuilt artefacts. This remains the default so existing unpinned +commands keep their rolling-update behaviour. Pin the suite explicitly when a +CI or production workflow needs a stable revision. + +## Workspace and checkout behaviour + +- Do not pass `--ref` while running inside a Whitaker source workspace. The + installer refuses this combination rather than checking out over the current + working tree. Run the installer from the consuming project or another + directory so it can use its managed clone. +- A successful pinned install checks out the managed clone at a detached + commit. This keeps the selected revision exact and allows the installer to + match a prebuilt archive to the resolved commit; a mismatch falls back to a + source build of that commit. +- A later unpinned install reattaches a detached managed clone to the remote + default branch before pulling. Run without `--no-update` when the clone + should return to the rolling default and receive updates. +- `--ref` composes with `--no-update`: the installer first tries to fetch the + requested ref and falls back to a ref or SHA already present in the clone if + the fetch is unavailable. This permits offline pinned installs without + changing the requested revision when it is already local. + +## Migration checklist + +1. Keep existing installer invocations unchanged if rolling suite updates are + intended. +2. For reproducible installs, add `--ref` with a release tag or commit SHA to + the installer command used by CI or release automation. +3. Move any pinned invocation out of the Whitaker source checkout and into the + consuming project or another directory. +4. After a temporary pin, run the installer without `--ref` and without + `--no-update` to reattach and update the managed clone to the rolling + default. + +For user-facing installation details, see the [User's Guide](../users-guide.md). +For the implementation-facing Git operation contracts, see the +[Developer's Guide](../developers-guide.md#public-git-operation-apis). diff --git a/docs/users-guide.md b/docs/users-guide.md index a84c8333..3f110c7b 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -75,6 +75,9 @@ environment-variable workaround. - `--skip-wrapper` — Skip wrapper script generation (prints `DYLINT_LIBRARY_PATH` instructions instead) - `--no-update` — Don't update existing repository clone +- `--ref ` — Build and stage the lint suite from a specific commit + of `leynos/whitaker` instead of the rolling default (see + [Pinning the suite](#pinning-the-suite)) ### Adding Whitaker to a project @@ -174,6 +177,47 @@ build experimental lints as individual libraries, combine it with require `--experimental`; without that opt-in the installer rejects the request before building anything. +### Pinning the suite + +By default the standalone installer tracks the moving `leynos/whitaker` default +branch (and its rolling prebuilt artefacts), so a push to `main` can change +lint behaviour on the next install. To hold the lint suite steady, pin it to an +explicit commit with `--ref`: + +```sh +whitaker-installer --ref v0.2.5 # a release tag +whitaker-installer --ref 1a2b3c4d # a commit SHA +``` + +`--ref` accepts any commit-ish that exists in the repository — a tag, a full or +abbreviated commit SHA, or a branch name. Tags and commit SHAs give +reproducible pins. A branch name is resolved to whatever commit it points at +when the installer runs, so it is *not* a reproducible pin; prefer a tag or SHA +for CI. Note that the installer version and the suite version are independent: +`cargo binstall whitaker-installer@0.2.5` pins only the installer, whereas +`--ref` pins the suite the installer builds. Tags exist for released versions, +but any commit-ish the repository contains is accepted; an unknown ref fails +with the underlying git error and stages nothing. + +The pin composes with the other flags: + +- **Prebuilt artefacts.** The rolling prebuilt archive only ever carries one + commit's libraries, so a pinned install reuses it only when the resolved + commit matches the archive's recorded commit; otherwise the installer builds + the pinned commit from source. Because the pinned commit supplies its own + `rust-toolchain.toml`, the staged libraries are named for that commit's + toolchain channel. +- **`--no-update`.** With `--ref --no-update` the installer skips the + default-branch update but still attempts to fetch the requested ref. If that + fetch fails, it falls back to a ref or SHA already available in the existing + clone, so offline pinned installs continue to work. + +`--ref` is refused when the current directory is itself a Whitaker workspace, +because checking out a commit there could destroy uncommitted work. Run the +installer from outside a checkout to pin the suite. A pinned install leaves the +managed clone on a detached commit; a later un-pinned `whitaker-installer` run +reattaches it to the default branch automatically before updating. + ## Lint Configuration Configure lint behaviour in `dylint.toml` at the workspace root: diff --git a/installer/Cargo.toml b/installer/Cargo.toml index 6d514e9e..9c3971d0 100644 --- a/installer/Cargo.toml +++ b/installer/Cargo.toml @@ -72,14 +72,17 @@ tempfile = { workspace = true } thiserror = { workspace = true } toml = { workspace = true } tracing = { workspace = true } +tracing-subscriber = { version = "0.3", features = ["env-filter"] } ureq = { workspace = true } wait-timeout = { workspace = true } zip = { workspace = true } zstd = { workspace = true } [dev-dependencies] +insta = { workspace = true } libc = { workspace = true } mockall = { workspace = true } +proptest = { workspace = true } rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } diff --git a/installer/src/cli.rs b/installer/src/cli.rs index 0ef5fbc5..bbbfbc35 100644 --- a/installer/src/cli.rs +++ b/installer/src/cli.rs @@ -9,6 +9,23 @@ use crate::resolution::EXPERIMENTAL_LINT_CRATES; use camino::Utf8PathBuf; use clap::{Parser, Subcommand}; +/// Validate a commit-ish supplied to `--ref` without normalizing it. +fn parse_git_ref(value: &str) -> Result { + if value.is_empty() { + return Err("ref must not be empty".to_owned()); + } + if value.starts_with('-') { + return Err("ref must not begin with '-'".to_owned()); + } + if value + .chars() + .any(|character| character.is_whitespace() || character.is_control()) + { + return Err("ref must not contain whitespace or control characters".to_owned()); + } + Ok(value.to_owned()) +} + /// Install Whitaker Dylint lint libraries. #[derive(Parser, Debug)] #[command(name = "whitaker-installer")] @@ -46,6 +63,8 @@ use clap::{Parser, Subcommand}; " $ whitaker-installer --individual-lints\n\n", " Include experimental lints in the suite:\n", " $ whitaker-installer --experimental\n\n", + " Pin the suite to a specific commit SHA or tag:\n", + " $ whitaker-installer --ref v0.2.5\n\n", " List installed lints:\n", " $ whitaker-installer list\n\n", " Preview without building:\n", @@ -136,6 +155,10 @@ pub struct InstallArgs { /// Skip prebuilt artefact download and build from source. #[arg(long = "build-only")] pub is_build_only: bool, + + /// Install the lint suite at a specific commit SHA or tag [default: rolling]. + #[arg(long = "ref", value_name = "REF", value_parser = parse_git_ref)] + pub git_ref: Option, } /// Arguments for the list command. @@ -219,6 +242,7 @@ impl Default for InstallArgs { skip_wrapper: false, no_update: false, is_build_only: false, + git_ref: None, } } } diff --git a/installer/src/cli_tests.rs b/installer/src/cli_tests.rs index 8b732bd2..54bb5ead 100644 --- a/installer/src/cli_tests.rs +++ b/installer/src/cli_tests.rs @@ -19,6 +19,48 @@ fn cli_parses_defaults() { assert!(!cli.install.skip_wrapper); assert!(!cli.install.no_update); assert!(!cli.install.is_build_only); + assert!(cli.install.git_ref.is_none()); +} + +#[test] +fn cli_parses_ref_flag_bare() { + let cli = Cli::parse_from(["whitaker-installer", "--ref", "v0.2.5"]); + assert_eq!(cli.install.git_ref.as_deref(), Some("v0.2.5")); +} + +#[test] +fn cli_parses_ref_flag_under_install_subcommand() { + let cli = Cli::parse_from(["whitaker-installer", "install", "--ref", "1a2b3c4d"]); + match cli.command { + Some(Command::Install(args)) => { + assert_eq!(args.git_ref.as_deref(), Some("1a2b3c4d")); + } + _ => panic!("expected Install command"), + } +} + +#[rstest] +#[case::empty("--ref=")] +#[case::leading_hyphen("--ref=-release")] +#[case::embedded_space("--ref=release candidate")] +#[case::leading_space("--ref= release-candidate")] +#[case::ansi_escape("--ref=release\u{1b}[31m")] +fn cli_rejects_invalid_ref_values(#[case] git_ref: &str) { + assert!( + Cli::try_parse_from(["whitaker-installer", git_ref]).is_err(), + "expected {git_ref:?} to be rejected" + ); +} + +#[rstest] +#[case::tag("v0.2.5")] +#[case::sha("1a2b3c4d")] +#[case::branch("release/candidate")] +#[case::revision_expression("main~2")] +fn cli_preserves_valid_ref_values(#[case] git_ref: &str) { + let cli = Cli::try_parse_from(["whitaker-installer", "--ref", git_ref]) + .expect("valid commit-ish should parse"); + assert_eq!(cli.install.git_ref.as_deref(), Some(git_ref)); } #[test] diff --git a/installer/src/diagnostics.rs b/installer/src/diagnostics.rs new file mode 100644 index 00000000..fdf3f7f3 --- /dev/null +++ b/installer/src/diagnostics.rs @@ -0,0 +1,15 @@ +//! Runtime diagnostics configuration for the installer binary. +//! +//! The binary installs one stderr tracing subscriber so operational Git and +//! workspace events are available through the standard `RUST_LOG` filter. + +use tracing_subscriber::EnvFilter; + +/// Installs the process-wide installer diagnostics subscriber. +pub(super) fn initialize() { + let filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("warn")); + let _ = tracing_subscriber::fmt() + .with_env_filter(filter) + .with_target(false) + .try_init(); +} diff --git a/installer/src/error.rs b/installer/src/error.rs index 93e45cdc..bbded449 100644 --- a/installer/src/error.rs +++ b/installer/src/error.rs @@ -105,6 +105,25 @@ pub enum InstallerError { reason: String, }, + /// Acquiring the managed-clone preparation lock failed. + #[error("failed to lock managed Whitaker workspace {path}: {source}")] + WorkspaceLock { + /// Sidecar lock file path. + path: Utf8PathBuf, + /// Underlying lock or file-system error. + #[source] + source: std::io::Error, + }, + + /// Pinning a ref is not supported for the current directory workspace. + #[error( + "cannot pin --ref {git_ref}: the current directory is itself a Whitaker workspace; run the installer from outside a checkout to pin the suite" + )] + RefUnsupported { + /// The requested ref that cannot be pinned in the current workspace. + git_ref: String, + }, + /// A Cargo.toml file could not be parsed during workspace detection. #[error("invalid Cargo.toml at {path}: {reason}")] InvalidCargoToml { @@ -231,6 +250,13 @@ impl Clone for InstallerError { Self::WorkspaceNotFound { reason } => Self::WorkspaceNotFound { reason: reason.clone(), }, + Self::WorkspaceLock { path, source } => Self::WorkspaceLock { + path: path.clone(), + source: clone_io_error(source), + }, + Self::RefUnsupported { git_ref } => Self::RefUnsupported { + git_ref: git_ref.clone(), + }, Self::InvalidCargoToml { path, reason } => Self::InvalidCargoToml { path: path.clone(), reason: reason.clone(), diff --git a/installer/src/git.rs b/installer/src/git.rs index f59a00e1..e12783fd 100644 --- a/installer/src/git.rs +++ b/installer/src/git.rs @@ -1,19 +1,31 @@ -//! Git operations for cloning and updating the Whitaker repository. +//! Git operations for managed Whitaker checkouts. //! -//! This module provides functions for managing the local Whitaker clone, -//! including initial cloning and subsequent updates. Operations have a -//! configurable timeout to prevent hangs on network issues. +//! This module clones and updates the managed repository, resolves requested +//! refs, performs detached pinned checkouts, and repairs a detached checkout +//! before an unpinned update. Workspace orchestration selects when to call +//! these helpers; the CLI reports the resulting action and pin progress. +//! Operations have a configurable timeout to prevent hangs on network issues. + +mod commit_sha; + +pub use commit_sha::CommitSha; use crate::error::{InstallerError, Result}; -use crate::workspace::WHITAKER_REPO_URL; use camino::Utf8Path; use std::process::{Command, Output, Stdio}; use std::time::Duration; +use tracing::{debug, warn}; use wait_timeout::ChildExt; /// Default timeout for git operations (5 minutes). const GIT_TIMEOUT: Duration = Duration::from_secs(300); +/// Repository URL for cloning Whitaker. +pub const WHITAKER_REPO_URL: &str = "https://github.com/leynos/whitaker"; + +/// Private ref used to identify the commit fetched for a requested pin. +const PINNED_REF: &str = "refs/whitaker/pinned-ref"; + /// Clones the Whitaker repository to the specified target directory. /// /// Creates the parent directories if they do not exist. The operation has @@ -28,21 +40,11 @@ pub fn clone_repository(target: &Utf8Path) -> Result<()> { std::fs::create_dir_all(parent)?; } - let output = run_git_with_timeout( + run_git_checked( &["clone", WHITAKER_REPO_URL, target.as_str()], None, "clone", - )?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(InstallerError::Git { - operation: "clone", - message: stderr.trim().to_owned(), - }); - } - - Ok(()) + ) } /// Updates an existing Whitaker repository by pulling the latest changes. @@ -54,17 +56,219 @@ pub fn clone_repository(target: &Utf8Path) -> Result<()> { /// /// Returns `InstallerError::Git` if the pull fails or times out. pub fn update_repository(repo: &Utf8Path) -> Result<()> { - let output = run_git_with_timeout(&["pull"], Some(repo), "pull")?; + run_git_checked(&["pull"], Some(repo), "pull") +} + +/// Resolves a commit-ish (SHA, tag, or branch) to a full commit SHA. +/// +/// Runs `git rev-parse --verify ^{commit}` in the repository so that +/// only expressions naming an existing commit succeed; annotated tags are +/// peeled to the commit they point at. +/// +/// # Errors +/// +/// Returns `InstallerError::Git` if the ref cannot be resolved or the command +/// times out. +pub fn resolve_commit(repo: &Utf8Path, refspec: &str) -> Result { + debug!( + operation = "rev-parse", + refspec, "resolving requested Git ref" + ); + let peeled = format!("{refspec}^{{commit}}"); + let output = + run_git_with_timeout(&["rev-parse", "--verify", &peeled], Some(repo), "rev-parse")?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr); return Err(InstallerError::Git { - operation: "pull", - message: stderr.trim().to_owned(), + operation: "rev-parse", + message: format!("could not resolve ref '{refspec}': {}", stderr.trim()), }); } - Ok(()) + let commit = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + CommitSha::try_from(commit.as_str()).map_err(|error| InstallerError::Git { + operation: "rev-parse", + message: format!("resolved ref '{refspec}' returned an invalid commit SHA: {error}"), + }) +} + +/// Fetches a specific ref from `origin` into the private pinned ref. +/// +/// Used to recover when a pinned ref cannot be resolved from the existing +/// clone. The requested ref is force-updated into a private local ref so the +/// returned commit is unambiguous. +/// +/// # Errors +/// +/// Returns `InstallerError::Git` if the fetch fails or times out. +pub fn fetch_ref(repo: &Utf8Path, refspec: &str) -> Result { + debug!( + operation = "fetch", + refspec, + attempt = "remote", + "fetching requested Git ref" + ); + let pinned_refspec = format!("+{refspec}:{PINNED_REF}"); + run_git_checked(&["fetch", "origin", &pinned_refspec], Some(repo), "fetch")?; + resolve_commit(repo, PINNED_REF) +} + +/// Checks out a commit as a detached HEAD. +/// +/// Runs `git checkout --detach `, leaving the working tree at exactly +/// the given commit without moving any branch. +/// +/// # Errors +/// +/// Returns `InstallerError::Git` if the checkout fails or times out. +pub fn checkout_detached(repo: &Utf8Path, commit: &CommitSha) -> Result<()> { + debug!( + operation = "checkout", + commit = commit.as_str(), + attempt = "detached", + "checking out pinned commit" + ); + run_git_checked( + &["checkout", "--detach", commit.as_str()], + Some(repo), + "checkout", + ) +} + +/// Returns the detached HEAD commit, or `None` when HEAD names a branch. +/// +/// # Errors +/// +/// Returns `InstallerError::Git` if HEAD is detached but cannot be resolved. +pub fn detached_head_commit(repo: &Utf8Path) -> Result> { + let symbolic = + run_git_with_timeout(&["symbolic-ref", "-q", "HEAD"], Some(repo), "symbolic-ref")?; + if symbolic.status.success() { + return Ok(None); + } + resolve_commit(repo, "HEAD").map(Some) +} + +/// Reattaches the repository to its default branch when HEAD is detached. +/// +/// A previous pinned install may leave the platform clone on a detached HEAD, +/// which makes a later `git pull` fail. This restores a branch checkout so that +/// subsequent updates succeed. It is a no-op when HEAD is already on a branch. +/// +/// The default branch is discovered from `origin/HEAD` +/// (`git rev-parse --abbrev-ref origin/HEAD`, e.g. `origin/main`); when an older +/// clone lacks that symbolic ref, `git remote set-head origin --auto` restores +/// it before retrying. +/// +/// # Errors +/// +/// Returns `InstallerError::Git` if the default branch cannot be determined or +/// checked out. +pub fn ensure_default_branch(repo: &Utf8Path) -> Result<()> { + // When HEAD already names a branch, there is nothing to reattach. + let symbolic = + run_git_with_timeout(&["symbolic-ref", "-q", "HEAD"], Some(repo), "symbolic-ref")?; + if symbolic.status.success() { + return Ok(()); + } + + debug!( + operation = "checkout", + attempt = "default_branch_recovery", + "reattaching detached checkout" + ); + let branch = match default_branch_name(repo)? { + Some(branch) => branch, + None => { + repair_default_branch_head(repo)?; + default_branch_name(repo)?.ok_or_else(|| InstallerError::Git { + operation: "rev-parse", + message: "could not determine default branch from origin/HEAD".to_owned(), + })? + } + }; + run_git_checked(&["checkout", &branch], Some(repo), "checkout") +} + +/// Discovers the remote default branch name (without the `origin/` prefix). +fn default_branch_name(repo: &Utf8Path) -> Result> { + let Some(branch) = read_default_branch(repo)? else { + return Ok(None); + }; + validate_default_branch(repo, &branch)?; + Ok(Some(branch)) +} + +/// Restores a missing `origin/HEAD` symbolic reference from the remote. +fn repair_default_branch_head(repo: &Utf8Path) -> Result<()> { + // An older clone may lack origin/HEAD; ask git to repopulate it before retrying. + debug!( + operation = "remote", + attempt = "repair_origin_head", + "repairing origin default branch reference" + ); + run_git_checked( + &["remote", "set-head", "origin", "--auto"], + Some(repo), + "remote", + ) +} + +/// Rejects a remote-head value that Git would parse as a checkout option. +fn validate_default_branch(repo: &Utf8Path, branch: &str) -> Result<()> { + let output = run_git_with_timeout( + &["check-ref-format", "--branch", branch], + Some(repo), + "check-ref-format", + )?; + if output.status.success() { + return Ok(()); + } + + Err(InstallerError::Git { + operation: "check-ref-format", + message: format!( + "invalid default branch from origin/HEAD: {}", + String::from_utf8_lossy(&output.stderr).trim() + ), + }) +} + +/// Reads `origin/HEAD` and returns the bare branch name, if present. +fn read_default_branch(repo: &Utf8Path) -> Result> { + let output = run_git_with_timeout( + &["rev-parse", "--abbrev-ref", "origin/HEAD"], + Some(repo), + "rev-parse", + )?; + if !output.status.success() { + return Ok(None); + } + + let value = String::from_utf8_lossy(&output.stdout).trim().to_owned(); + Ok(value + .strip_prefix("origin/") + .map(ToOwned::to_owned) + .filter(|branch| !branch.is_empty())) +} + +/// Runs a Git command whose successful output is intentionally discarded. +fn run_git_checked( + args: &[&str], + working_dir: Option<&Utf8Path>, + operation: &'static str, +) -> Result<()> { + let output = run_git_with_timeout(args, working_dir, operation)?; + if output.status.success() { + return Ok(()); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + Err(InstallerError::Git { + operation, + message: stderr.trim().to_owned(), + }) } /// Runs a git command with a timeout. @@ -133,6 +337,11 @@ fn run_git_with_timeout( } None => { // Timeout - kill the process and wait for threads to finish + warn!( + operation, + timeout_seconds = GIT_TIMEOUT.as_secs(), + "git operation timed out" + ); let _ = child.kill(); let _ = child.wait(); let _ = stdout_thread.join(); @@ -149,28 +358,5 @@ fn run_git_with_timeout( } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn clone_repository_error_includes_operation() { - let err = InstallerError::Git { - operation: "clone", - message: "test error".to_owned(), - }; - let msg = err.to_string(); - assert!(msg.contains("clone")); - assert!(msg.contains("test error")); - } - - #[test] - fn update_repository_error_includes_operation() { - let err = InstallerError::Git { - operation: "pull", - message: "not a git repository".to_owned(), - }; - let msg = err.to_string(); - assert!(msg.contains("pull")); - assert!(msg.contains("not a git repository")); - } -} +#[path = "git_tests.rs"] +mod tests; diff --git a/installer/src/git/commit_sha.rs b/installer/src/git/commit_sha.rs new file mode 100644 index 00000000..631bc660 --- /dev/null +++ b/installer/src/git/commit_sha.rs @@ -0,0 +1,73 @@ +//! Full Git commit object IDs returned by repository operations. +//! +//! The Git adapter constructs this type only after `git rev-parse` has +//! resolved a commit-ish to a complete object ID. Callers retain the typed +//! value through checkout and prebuilt provenance validation. + +use std::fmt; +use thiserror::Error; + +/// A validated, full lowercase hexadecimal Git commit object ID. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct CommitSha(String); + +/// An error returned when a value is not a full Git commit object ID. +#[derive(Debug, Error)] +#[error("invalid full Git commit SHA: {reason}")] +pub struct CommitShaError { + reason: &'static str, +} + +impl CommitSha { + /// Returns the full commit object ID as a string slice. + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl TryFrom<&str> for CommitSha { + type Error = CommitShaError; + + fn try_from(value: &str) -> Result { + if value.len() != 40 { + return Err(CommitShaError { + reason: "SHA must contain exactly 40 characters", + }); + } + if !value + .bytes() + .all(|byte| byte.is_ascii_digit() || matches!(byte, b'a'..=b'f')) + { + return Err(CommitShaError { + reason: "SHA must contain only lowercase hexadecimal characters", + }); + } + Ok(Self(value.to_owned())) + } +} + +impl fmt::Display for CommitSha { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(&self.0) + } +} + +#[cfg(test)] +mod tests { + //! Validates `CommitSha` parsing and rejection behaviour. + + use super::CommitSha; + use rstest::rstest; + + #[rstest] + #[case::valid("abc12340000000000000000000000000000000ab", true)] + #[case::too_short("abc1234", false)] + #[case::uppercase("ABC12340000000000000000000000000000000AB", false)] + #[case::non_hex("abc123g0000000000000000000000000000000000", false)] + fn validates_full_lowercase_hex_shas(#[case] value: &str, #[case] expected: bool) { + let result = CommitSha::try_from(value); + + assert_eq!(result.is_ok(), expected); + } +} diff --git a/installer/src/git_tests.rs b/installer/src/git_tests.rs new file mode 100644 index 00000000..add023cd --- /dev/null +++ b/installer/src/git_tests.rs @@ -0,0 +1,363 @@ +//! Real-Git regression tests for clone updates and pinned checkouts. + +use super::*; +use camino::Utf8PathBuf; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use rstest::{fixture, rstest}; +use std::process::Command; +use tempfile::TempDir; + +/// Run a Git command in `dir`, asserting success, and return trimmed stdout. +fn git(dir: &Utf8Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(dir.as_std_path()) + .output() + .expect("failed to spawn git"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_owned() +} + +/// Commit the given file content in `dir` and return the resulting SHA. +fn commit_file(dir: &Utf8Path, name: &str, contents: &str, message: &str) -> String { + let directory = + Dir::open_ambient_dir(dir, ambient_authority()).expect("open fixture directory"); + directory.write(name, contents).expect("write fixture file"); + git(dir, &["add", "."]); + git( + dir, + &[ + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + "commit", + "-m", + message, + ], + ); + git(dir, &["rev-parse", "HEAD"]) +} + +/// A source repository plus a clone of it, with recorded commit SHAs. +struct GitFixture { + _source: TempDir, + _clone: TempDir, + clone: Utf8PathBuf, + first: String, + second: String, +} + +/// Build a source repo (two commits, tag `v1` on the first) and clone it. +#[fixture] +fn git_fixture() -> GitFixture { + let source = TempDir::new().expect("source temp dir"); + let source_path = Utf8PathBuf::try_from(source.path().to_owned()).expect("UTF-8 source path"); + git(&source_path, &["init", "-b", "main"]); + let first = commit_file(&source_path, "a.txt", "one", "first"); + git(&source_path, &["tag", "v1"]); + let second = commit_file(&source_path, "b.txt", "two", "second"); + + let clone = TempDir::new().expect("clone temp dir"); + let clone_path = Utf8PathBuf::try_from(clone.path().to_owned()).expect("UTF-8 clone path"); + let output = Command::new("git") + .args(["clone", source_path.as_str(), clone_path.as_str()]) + .output() + .expect("failed to spawn git clone"); + assert!( + output.status.success(), + "git clone failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + GitFixture { + _source: source, + _clone: clone, + clone: clone_path, + first, + second, + } +} + +#[rstest] +fn resolve_commit_resolves_tag_branch_and_sha(git_fixture: GitFixture) { + assert_eq!( + resolve_commit(&git_fixture.clone, "v1") + .expect("resolve tag") + .as_str(), + git_fixture.first.as_str() + ); + assert_eq!( + resolve_commit(&git_fixture.clone, "main") + .expect("resolve branch") + .as_str(), + git_fixture.second.as_str() + ); + assert_eq!( + resolve_commit(&git_fixture.clone, &git_fixture.second) + .expect("resolve sha") + .as_str(), + git_fixture.second.as_str() + ); +} + +#[rstest] +fn resolve_commit_errors_on_garbage(git_fixture: GitFixture) { + let err = + resolve_commit(&git_fixture.clone, "definitely-not-a-ref").expect_err("expected error"); + assert!(matches!(err, InstallerError::Git { .. }), "got {err:?}"); +} + +#[rstest] +fn checkout_detached_leaves_head_at_commit(git_fixture: GitFixture) { + let commit = resolve_commit(&git_fixture.clone, "v1").expect("resolve tag"); + checkout_detached(&git_fixture.clone, &commit).expect("checkout detached"); + assert_eq!( + git(&git_fixture.clone, &["rev-parse", "HEAD"]), + git_fixture.first + ); + // A detached HEAD has no symbolic ref. + let symbolic = Command::new("git") + .args(["symbolic-ref", "-q", "HEAD"]) + .current_dir(git_fixture.clone.as_std_path()) + .output() + .expect("spawn symbolic-ref"); + assert!(!symbolic.status.success(), "expected detached HEAD"); +} + +#[rstest] +fn unpinned_no_update_preserves_detached_commit_gate(git_fixture: GitFixture) { + crate::workspace::pin_to_ref(&git_fixture.clone, "v1").expect("pin initial install"); + + let checkout = crate::workspace::finalize_workspace_checkout( + git_fixture.clone.clone(), + None, + crate::workspace::WorkspaceAction::UseExisting(git_fixture.clone.clone()), + ) + .expect("reuse detached checkout without updating"); + + assert_eq!(checkout.pinned_commit, None); + assert_eq!( + checkout.detached_commit.as_ref().map(CommitSha::as_str), + Some(git_fixture.first.as_str()) + ); + assert_eq!( + checkout.expected_git_sha().map(CommitSha::as_str), + Some(git_fixture.first.as_str()) + ); +} + +#[rstest] +fn pinned_checkout_reattaches_for_unpinned_update(git_fixture: GitFixture) { + let pinned_commit = + crate::workspace::pin_to_ref(&git_fixture.clone, "v1").expect("pin checkout to v1"); + assert_eq!(pinned_commit.as_str(), git_fixture.first); + assert_eq!( + git(&git_fixture.clone, &["rev-parse", "--abbrev-ref", "HEAD"]), + "HEAD" + ); + + let source = Utf8PathBuf::try_from(git_fixture._source.path().to_owned()).expect("UTF-8 path"); + let third = commit_file(&source, "c.txt", "three", "third"); + + ensure_default_branch(&git_fixture.clone).expect("reattach to default branch"); + update_repository(&git_fixture.clone).expect("update after reattach"); + + assert_eq!( + git(&git_fixture.clone, &["symbolic-ref", "HEAD"]), + "refs/heads/main" + ); + assert_eq!(git(&git_fixture.clone, &["rev-parse", "HEAD"]), third); +} + +#[rstest] +fn ensure_default_branch_is_noop_on_a_branch(git_fixture: GitFixture) { + ensure_default_branch(&git_fixture.clone).expect("noop on branch"); + assert_eq!( + git(&git_fixture.clone, &["symbolic-ref", "HEAD"]), + "refs/heads/main" + ); +} + +#[rstest] +fn default_branch_name_does_not_repair_missing_remote_head(git_fixture: GitFixture) { + git( + &git_fixture.clone, + &["symbolic-ref", "--delete", "refs/remotes/origin/HEAD"], + ); + + assert_eq!( + default_branch_name(&git_fixture.clone).expect("query default branch"), + None + ); + let remote_head = Command::new("git") + .args(["symbolic-ref", "refs/remotes/origin/HEAD"]) + .current_dir(git_fixture.clone.as_std_path()) + .output() + .expect("query missing origin HEAD"); + assert!( + !remote_head.status.success(), + "query must not repair origin/HEAD" + ); +} + +#[rstest] +fn ensure_default_branch_repairs_missing_remote_head(git_fixture: GitFixture) { + let commit = resolve_commit(&git_fixture.clone, "v1").expect("resolve tag"); + checkout_detached(&git_fixture.clone, &commit).expect("checkout detached"); + git( + &git_fixture.clone, + &["symbolic-ref", "--delete", "refs/remotes/origin/HEAD"], + ); + + ensure_default_branch(&git_fixture.clone).expect("repair and reattach default branch"); + + assert_eq!( + git(&git_fixture.clone, &["symbolic-ref", "HEAD"]), + "refs/heads/main" + ); + assert_eq!( + git( + &git_fixture.clone, + &["symbolic-ref", "refs/remotes/origin/HEAD"] + ), + "refs/remotes/origin/main" + ); +} + +#[rstest] +fn default_branch_validation_rejects_option_like_ref(git_fixture: GitFixture) { + let err = validate_default_branch(&git_fixture.clone, "--orphan=attacker") + .expect_err("option-like default branch must be rejected"); + + assert!( + matches!( + err, + InstallerError::Git { + operation: "check-ref-format", + .. + } + ), + "got {err:?}" + ); +} + +#[rstest] +fn fetch_ref_retrieves_a_new_tag(git_fixture: GitFixture) { + // Add a third commit and tag it in the source, after the clone was made. + let source = Utf8PathBuf::try_from(git_fixture._source.path().to_owned()).expect("UTF-8 path"); + let third = commit_file(&source, "c.txt", "three", "third"); + git(&source, &["tag", "v2"]); + commit_file(&source, "d.txt", "four", "unrelated tag commit"); + git(&source, &["tag", "unrelated"]); + + // The clone cannot resolve the new tag until it fetches. + assert!(resolve_commit(&git_fixture.clone, "v2").is_err()); + let fetched = fetch_ref(&git_fixture.clone, "v2").expect("fetch new tag"); + assert_eq!(fetched.as_str(), third); + assert_eq!( + resolve_commit(&git_fixture.clone, PINNED_REF) + .expect("resolve private pinned ref") + .as_str(), + third + ); + assert_eq!( + resolve_commit(&git_fixture.clone, "v2") + .expect("resolve v2") + .as_str(), + third + ); + assert!( + resolve_commit(&git_fixture.clone, "unrelated").is_err(), + "fetching v2 must not transfer unrelated tags" + ); +} + +#[rstest] +fn pin_to_ref_fetches_and_checks_out_a_new_remote_branch(git_fixture: GitFixture) { + let source = Utf8PathBuf::try_from(git_fixture._source.path().to_owned()).expect("UTF-8 path"); + git(&source, &["checkout", "-b", "release-candidate"]); + let branch_commit = commit_file(&source, "c.txt", "three", "branch commit"); + + assert!(resolve_commit(&git_fixture.clone, "release-candidate").is_err()); + let pinned_commit = crate::workspace::pin_to_ref(&git_fixture.clone, "release-candidate") + .expect("fetch and pin new remote branch"); + + assert_eq!(pinned_commit.as_str(), branch_commit); + assert_eq!( + git(&git_fixture.clone, &["rev-parse", "HEAD"]), + branch_commit + ); + assert_eq!( + git(&git_fixture.clone, &["rev-parse", "--abbrev-ref", "HEAD"]), + "HEAD" + ); +} + +#[rstest] +fn pin_to_ref_prefers_an_updated_remote_branch(git_fixture: GitFixture) { + let source = Utf8PathBuf::try_from(git_fixture._source.path().to_owned()).expect("UTF-8 path"); + let third = commit_file(&source, "c.txt", "three", "third"); + + let pinned_commit = crate::workspace::pin_to_ref(&git_fixture.clone, "main") + .expect("fetch and pin updated main"); + + assert_eq!(pinned_commit.as_str(), third); + assert_eq!(git(&git_fixture.clone, &["rev-parse", "HEAD"]), third); +} + +#[rstest] +fn pin_to_ref_falls_back_to_a_local_ref_offline(git_fixture: GitFixture) { + git(&git_fixture.clone, &["remote", "remove", "origin"]); + + let pinned_commit = crate::workspace::pin_to_ref(&git_fixture.clone, "v1") + .expect("pin locally available tag while offline"); + + assert_eq!(pinned_commit.as_str(), git_fixture.first); + assert_eq!( + git(&git_fixture.clone, &["rev-parse", "--abbrev-ref", "HEAD"]), + "HEAD" + ); +} + +#[test] +fn clone_repository_error_includes_operation() { + let target = TempDir::new().expect("clone target temp dir"); + let target_path = Utf8PathBuf::try_from(target.path().to_owned()).expect("UTF-8 target path"); + let target_dir = Dir::open_ambient_dir(&target_path, ambient_authority()) + .expect("open clone target directory"); + target_dir + .write("occupied", b"occupied") + .expect("write target file"); + + let err = clone_repository(&target_path).expect_err("clone into non-empty target should fail"); + + let InstallerError::Git { operation, message } = err else { + panic!("expected Git error, got {err:?}"); + }; + assert_eq!(operation, "clone"); + assert!(!message.is_empty(), "expected Git stderr"); +} + +#[test] +fn update_repository_error_includes_operation() { + let repo = TempDir::new().expect("non-repository temp dir"); + let repo_path = Utf8PathBuf::try_from(repo.path().to_owned()).expect("UTF-8 path"); + + let err = update_repository(&repo_path).expect_err("pull outside a repository should fail"); + + let InstallerError::Git { operation, message } = err else { + panic!("expected Git error, got {err:?}"); + }; + assert_eq!(operation, "pull"); + assert!( + message.contains("not a git repository"), + "stderr: {message}" + ); +} diff --git a/installer/src/install_flow.rs b/installer/src/install_flow.rs index 54bf69a1..998b8f8b 100644 --- a/installer/src/install_flow.rs +++ b/installer/src/install_flow.rs @@ -20,11 +20,30 @@ use whitaker_installer::deps::{ use whitaker_installer::deps::{DependencyInstallOptions, install_dylint_tools_with_options}; use whitaker_installer::dirs::BaseDirs; use whitaker_installer::error::{InstallerError, Result}; +use whitaker_installer::git::CommitSha; use whitaker_installer::install_metrics::{InstallMode, RecordOutcome, record_install}; use whitaker_installer::output::write_stderr_line; use whitaker_installer::prebuilt::{PrebuiltConfig, PrebuiltResult, attempt_prebuilt}; use whitaker_installer::prebuilt_path::prebuilt_library_dir; use whitaker_installer::resolution::{EXPERIMENTAL_LINT_CRATES, LINT_CRATES, SUITE_CRATE}; +use whitaker_installer::workspace::{WorkspaceAction, ensure_ref_allowed}; + +/// Validates a ref before invoking the dependency installer. +pub(crate) fn ensure_dependencies_after_ref_validation( + args: &InstallArgs, + action: &WorkspaceAction, + stderr: &mut dyn Write, + ensure_dependencies: F, +) -> Result<()> +where + F: FnOnce(bool, &mut dyn Write) -> Result<()>, +{ + ensure_ref_allowed(action, args.git_ref.as_deref())?; + if !args.skip_deps { + ensure_dependencies(args.quiet, stderr)?; + } + Ok(()) +} pub(crate) fn ensure_dylint_tools_core( quiet: bool, @@ -83,6 +102,9 @@ pub(crate) struct PrebuiltInstallationContext<'a> { pub(crate) requested_crates: &'a [CrateName], /// Toolchain channel resolved for this install. pub(crate) toolchain_channel: &'a str, + /// Full commit ID for a requested pin or inherited detached `--no-update` + /// checkout, used to keep the prebuilt SHA gate active in either case. + pub(crate) expected_git_sha: Option<&'a CommitSha>, } /// Context for recording one successful install in aggregate metrics. @@ -181,6 +203,7 @@ fn try_prebuilt_installation_with( toolchain: context.toolchain_channel, destination_dir: &destination_dir, quiet: context.args.quiet, + expected_git_sha: context.expected_git_sha, }; let PrebuiltResult::Success { staging_path } = attempt_prebuilt(&prebuilt_config, stderr) diff --git a/installer/src/install_flow/tests.rs b/installer/src/install_flow/tests.rs index c520829a..983c4f4b 100644 --- a/installer/src/install_flow/tests.rs +++ b/installer/src/install_flow/tests.rs @@ -29,6 +29,7 @@ impl BaseDirs for TestBaseDirs { } static PRUNE_HOOK_CALLED: AtomicBool = AtomicBool::new(false); +const PINNED_COMMIT: &str = "abc12340000000000000000000000000000000ab"; fn stub_detect_host_target() -> Result { Ok("x86_64-unknown-linux-gnu".to_owned()) @@ -48,6 +49,20 @@ fn stub_attempt_prebuilt(_config: &PrebuiltConfig<'_>, _stderr: &mut dyn Write) } } +fn stub_reject_mismatched_pinned_prebuilt( + config: &PrebuiltConfig<'_>, + _stderr: &mut dyn Write, +) -> PrebuiltResult { + assert_eq!( + config.expected_git_sha.map(CommitSha::as_str), + Some(PINNED_COMMIT), + "the resolved pinned commit must reach prebuilt validation" + ); + PrebuiltResult::Fallback { + reason: "git SHA mismatch: manifest has deadbeef, pinned commit is abc1234".to_owned(), + } +} + fn stub_prune_prebuilt_libraries( _staging_path: &Utf8Path, _toolchain_channel: &str, @@ -173,6 +188,7 @@ fn try_prebuilt_installation_prune_error_falls_back_to_local_build() { dirs: &dirs, requested_crates: &requested_crates, toolchain_channel: "nightly-2026-05-28", + expected_git_sha: None, }; let mut stderr = Vec::new(); @@ -206,3 +222,37 @@ fn try_prebuilt_installation_prune_error_falls_back_to_local_build() { "fallback message should be emitted, stderr: {stderr}" ); } + +#[test] +fn pinned_commit_provenance_reaches_prebuilt_validation() { + let dirs = TestBaseDirs { + data_dir: Some(PathBuf::from("/tmp/whitaker-test-data")), + }; + let args = InstallArgs::default(); + let requested_crates = vec![CrateName::from(SUITE_CRATE)]; + let expected_git_sha = CommitSha::try_from(PINNED_COMMIT).expect("valid pinned commit"); + let context = PrebuiltInstallationContext { + args: &args, + dirs: &dirs, + requested_crates: &requested_crates, + toolchain_channel: "nightly-2026-05-28", + expected_git_sha: Some(&expected_git_sha), + }; + + let mut stderr = Vec::new(); + let result = try_prebuilt_installation_with( + &context, + &mut stderr, + PrebuiltInstallationHooks { + detect_host_target: stub_detect_host_target, + resolve_destination_dir: stub_resolve_destination_dir, + attempt_prebuilt: stub_reject_mismatched_pinned_prebuilt, + prune_prebuilt_libraries: stub_prune_prebuilt_libraries, + }, + ); + + assert!( + matches!(result, Ok(None)), + "a mismatched pinned manifest must fall back to source compilation" + ); +} diff --git a/installer/src/lib.rs b/installer/src/lib.rs index 94e14c53..d0c0d4b3 100644 --- a/installer/src/lib.rs +++ b/installer/src/lib.rs @@ -69,6 +69,7 @@ pub mod test_support; pub mod toolchain; pub mod version; pub mod workspace; +mod workspace_lock; pub mod wrapper; #[cfg(test)] diff --git a/installer/src/main.rs b/installer/src/main.rs index 1e9c2665..e33854b5 100644 --- a/installer/src/main.rs +++ b/installer/src/main.rs @@ -4,14 +4,17 @@ //! After installation, it prints shell configuration snippets for enabling //! library discovery. +mod diagnostics; mod install_flow; mod staged_suite; +mod workspace_progress; #[cfg(test)] use crate::install_flow::ensure_dylint_tools_with_options; use crate::install_flow::{ MetricsWriteContext, PrebuiltInstallationContext, detect_host_target, - ensure_dylint_tools_with_executor, try_prebuilt_installation, write_install_metrics, + ensure_dependencies_after_ref_validation, ensure_dylint_tools_with_executor, + try_prebuilt_installation, write_install_metrics, }; use camino::{Utf8Path, Utf8PathBuf}; use clap::Parser; @@ -22,6 +25,7 @@ use whitaker_installer::crate_name::CrateName; use whitaker_installer::deps::SystemCommandExecutor; use whitaker_installer::dirs::{BaseDirs, SystemBaseDirs}; use whitaker_installer::error::{InstallerError, Result}; +use whitaker_installer::git::CommitSha; use whitaker_installer::install_metrics::InstallMode; use whitaker_installer::list::{determine_target_dir, run_list}; use whitaker_installer::output::{DryRunInfo, ShellSnippet, write_stderr_line}; @@ -31,9 +35,11 @@ use whitaker_installer::resolution::{ CrateResolutionOptions, resolve_crates, validate_crate_names, }; use whitaker_installer::toolchain::Toolchain; +use whitaker_installer::workspace::{WorkspaceAction, WorkspaceCheckout}; use whitaker_installer::wrapper::{generate_wrapper_scripts, path_instructions}; fn main() { + diagnostics::initialize(); let cli = Cli::parse(); let mut stdout = std::io::stdout(); let mut stderr = std::io::stderr(); @@ -75,6 +81,7 @@ fn try_fast_path_installation( dirs: context.dirs, requested_crates: context.requested_crates, toolchain_channel: context.toolchain.channel(), + expected_git_sha: context.expected_git_sha, }; if let Some(staging_path) = try_prebuilt_installation(&prebuilt_context, stderr)? { return Ok(Some((staging_path, InstallMode::Download))); @@ -91,9 +98,8 @@ fn try_fast_path_installation( /// Runs the install command to build and stage lint libraries. /// -/// Workflow: (1) check/install Dylint dependencies, (2) locate/clone workspace, -/// (3) resolve crates from CLI flags, (4) build in release mode, (5) stage -/// libraries with toolchain-suffixed names, (6) generate wrapper script. +/// Validates the request, prepares dependencies and a workspace, then installs +/// the selected lint libraries and wrapper scripts. /// /// # Errors /// @@ -106,13 +112,11 @@ fn run_install(args: &InstallArgs, stderr: &mut dyn Write) -> Result<()> { return run_dry(args, &dirs, stderr); } let install_started = Instant::now(); - // Step 1: Check and install Dylint dependencies if needed - if !args.skip_deps { - ensure_dylint_tools(args.quiet, stderr)?; - } - // Step 2: Ensure workspace is available (clone if needed) - let workspace_root = ensure_whitaker_workspace(args, &dirs, stderr)?; - // Step 3: Resolve crates and toolchain + // Reject unsafe pin requests before dependency installation can mutate the host. + validate_ref_then_ensure_dependencies(args, &dirs, stderr, ensure_dylint_tools)?; + let workspace = ensure_whitaker_workspace(args, &dirs, stderr)?; + let expected_git_sha = workspace.expected_git_sha().cloned(); + let workspace_root = workspace.root; let requested_crates = resolve_requested_crates(args)?; let toolchain = resolve_toolchain(&workspace_root, args.toolchain.as_deref())?; ensure_toolchain_installed( @@ -122,13 +126,13 @@ fn run_install(args: &InstallArgs, stderr: &mut dyn Write) -> Result<()> { stderr, )?; let target_dir = determine_target_dir(args.target_dir.as_deref())?; - // Step 3.5: Attempt prebuilt download or staged-suite fast path. let fast_path_context = FastPathContext { args, dirs: &dirs, requested_crates: &requested_crates, toolchain: &toolchain, target_dir: &target_dir, + expected_git_sha: expected_git_sha.as_ref(), }; if let Some((staging_path, install_mode)) = try_fast_path_installation(&fast_path_context, stderr)? @@ -151,7 +155,6 @@ fn run_install(args: &InstallArgs, stderr: &mut dyn Write) -> Result<()> { experimental: args.experimental, quiet: args.quiet, }; - // Step 4: Build and stage let build_results = perform_build(&context, &requested_crates, stderr)?; let staging_path = stage_libraries(&context, &build_results, stderr)?; // Step 5: Generate wrapper scripts if requested @@ -167,9 +170,16 @@ fn run_install(args: &InstallArgs, stderr: &mut dyn Write) -> Result<()> { /// Runs in dry-run mode, showing configuration without side effects. fn run_dry(args: &InstallArgs, dirs: &dyn BaseDirs, stderr: &mut dyn Write) -> Result<()> { - use whitaker_installer::workspace::resolve_workspace_path; + use whitaker_installer::workspace::{ensure_ref_allowed, resolve_workspace_action}; - let workspace_root = resolve_workspace_path(dirs)?; + let action = resolve_workspace_action(dirs, !args.no_update)?; + ensure_ref_allowed(&action, args.git_ref.as_deref())?; + let workspace_root = match action { + WorkspaceAction::UseCurrentDir(dir) + | WorkspaceAction::CloneTo(dir) + | WorkspaceAction::UpdateAt(dir) + | WorkspaceAction::UseExisting(dir) => dir, + }; let requested_crates = resolve_requested_crates(args)?; let toolchain = resolve_toolchain(&workspace_root, args.toolchain.as_deref())?; toolchain.verify_installed()?; @@ -185,11 +195,28 @@ fn run_dry(args: &InstallArgs, dirs: &dyn BaseDirs, stderr: &mut dyn Write) -> R no_update: args.no_update, jobs: args.jobs, crates: &requested_crates, + git_ref: args.git_ref.as_deref(), }; write_stderr_line(stderr, info.display_text()); Ok(()) } +/// Validates a pin request before permitting dependency installation. +fn validate_ref_then_ensure_dependencies( + args: &InstallArgs, + dirs: &dyn BaseDirs, + stderr: &mut dyn Write, + ensure_dependencies: F, +) -> Result<()> +where + F: FnOnce(bool, &mut dyn Write) -> Result<()>, +{ + use whitaker_installer::workspace::resolve_workspace_action; + + let action = resolve_workspace_action(dirs, !args.no_update)?; + ensure_dependencies_after_ref_validation(args, &action, stderr, ensure_dependencies) +} + fn determine_dry_run_target_dir( args: &InstallArgs, dirs: &dyn BaseDirs, @@ -217,34 +244,14 @@ fn ensure_whitaker_workspace( args: &InstallArgs, dirs: &dyn BaseDirs, stderr: &mut dyn Write, -) -> Result { - use whitaker_installer::workspace::{ - WorkspaceAction, clone_directory, decide_workspace_action, ensure_workspace, - }; - - if !args.quiet - && let Some(clone_dir) = clone_directory(dirs) - { - let cwd = std::env::current_dir() - .ok() - .and_then(|p| Utf8PathBuf::try_from(p).ok()); - - let Some(cwd) = cwd else { - return ensure_workspace(dirs, !args.no_update); - }; - - match decide_workspace_action(&cwd, &clone_dir, !args.no_update) { - WorkspaceAction::CloneTo(dir) => { - write_stderr_line(stderr, format!("Cloning Whitaker repository to {dir}...")); - } - WorkspaceAction::UpdateAt(dir) => { - write_stderr_line(stderr, format!("Updating Whitaker repository at {dir}...")); - } - WorkspaceAction::UseCurrentDir(_) | WorkspaceAction::UseExisting(_) => {} - } - } +) -> Result { + use whitaker_installer::workspace::ensure_workspace; - ensure_workspace(dirs, !args.no_update) + let git_ref = args.git_ref.as_deref(); + let checkout = ensure_workspace(dirs, !args.no_update, git_ref)?; + workspace_progress::report_workspace_progress(args, &checkout, stderr); + workspace_progress::report_pinned_checkout(args.quiet, git_ref, &checkout, stderr); + Ok(checkout) } /// Detects or overrides the toolchain, then verifies it is installed. @@ -307,6 +314,7 @@ struct FastPathContext<'a> { requested_crates: &'a [CrateName], toolchain: &'a Toolchain, target_dir: &'a Utf8PathBuf, + expected_git_sha: Option<&'a CommitSha>, } /// Finalize installation and record aggregate installer metrics. diff --git a/installer/src/output.rs b/installer/src/output.rs index 53bc593f..4ccc9015 100644 --- a/installer/src/output.rs +++ b/installer/src/output.rs @@ -104,6 +104,7 @@ pub fn success_message(count: usize, target_dir: &Utf8Path) -> String { /// no_update: false, /// jobs: None, /// crates: &crates, +/// git_ref: None, /// }; /// /// let output = info.display_text(); @@ -132,6 +133,8 @@ pub struct DryRunInfo<'a> { pub jobs: Option, /// Crates to be built. pub crates: &'a [CrateName], + /// The commit-ish the suite is pinned to, if any. + pub git_ref: Option<&'a str>, } impl DryRunInfo<'_> { @@ -151,6 +154,10 @@ impl DryRunInfo<'_> { format!("No update: {}", self.no_update), ]; + if let Some(git_ref) = self.git_ref { + lines.push(format!("Pinned ref: {git_ref}")); + } + if let Some(jobs) = self.jobs { lines.push(format!("Parallel jobs: {jobs}")); } @@ -211,6 +218,37 @@ mod tests { assert!(display.contains("PowerShell")); } + fn dry_run_info<'a>(git_ref: Option<&'a str>, crates: &'a [CrateName]) -> DryRunInfo<'a> { + DryRunInfo { + workspace_root: Utf8Path::new("/home/user/whitaker"), + toolchain: "nightly-2025-01-15", + target_dir: Utf8Path::new("/home/user/.local/share/dylint/lib"), + verbosity: 0, + quiet: false, + skip_deps: false, + skip_wrapper: false, + no_update: false, + jobs: None, + crates, + git_ref, + } + } + + #[rstest] + #[case::default(None, None, "dry_run_display_matches_default_format")] + #[case::pinned(Some("v0.2.5"), Some(4), "dry_run_display_matches_pinned_format")] + fn dry_run_display_matches_expected_format( + #[case] git_ref: Option<&str>, + #[case] jobs: Option, + #[case] snapshot_name: &str, + ) { + let crates = vec![CrateName::from("whitaker_suite")]; + let mut info = dry_run_info(git_ref, &crates); + info.jobs = jobs; + + insta::assert_snapshot!(snapshot_name, info.display_text()); + } + #[rstest] #[case::singular(1, "1 lint library")] #[case::plural(5, "5 lint libraries")] diff --git a/installer/src/prebuilt.rs b/installer/src/prebuilt.rs index 3e4fab24..100dbe2d 100644 --- a/installer/src/prebuilt.rs +++ b/installer/src/prebuilt.rs @@ -20,6 +20,7 @@ use crate::artefact::packaging::compute_sha256; use crate::artefact::packaging_error::PackagingError; use crate::artefact::verification::VerificationPolicy; use crate::builder::{library_extension, library_prefix}; +use crate::git::CommitSha; use crate::output::write_stderr_line; /// The outcome of a prebuilt download attempt. @@ -52,6 +53,12 @@ pub struct PrebuiltConfig<'a> { pub destination_dir: &'a Utf8Path, /// When true, suppress progress output. pub quiet: bool, + /// When set, require an exact full-object-ID match in the manifest. + /// + /// Pinned installs may only reuse a prebuilt artefact whose manifest names + /// the resolved full commit SHA exactly; abbreviated provenance falls back + /// to a source build. Rolling installations leave this unset. + pub expected_git_sha: Option<&'a CommitSha>, } /// Internal error type for the prebuilt pipeline. @@ -72,6 +79,9 @@ enum PrebuiltError { #[error("target mismatch: manifest has {manifest}, expected {expected}")] TargetMismatch { manifest: String, expected: String }, + #[error("git SHA mismatch: manifest has {manifest}, pinned commit is {expected}")] + GitShaMismatch { manifest: String, expected: String }, + #[error("checksum mismatch: manifest={expected}, actual={actual}")] ChecksumMismatch { expected: String, actual: String }, @@ -138,6 +148,7 @@ fn run_pipeline( let manifest = parse_manifest(&manifest_json)?; validate_toolchain(&manifest, config.toolchain)?; validate_target(&manifest, config.target)?; + validate_git_sha(&manifest, config.expected_git_sha)?; // Step 3: Derive archive filename and download. let archive_filename = derive_archive_filename(&manifest); @@ -197,6 +208,27 @@ fn validate_target(manifest: &Manifest, expected: &str) -> Result<(), PrebuiltEr Ok(()) } +/// Validate exact full-object-ID provenance for a pinned installation. +/// +/// An abbreviated manifest SHA is suitable for rolling installations only. +/// Pinned installations require a full object ID equal to the resolved commit. +/// When no commit is pinned this is a no-op, preserving rolling behaviour. +fn validate_git_sha( + manifest: &Manifest, + expected: Option<&CommitSha>, +) -> Result<(), PrebuiltError> { + let Some(expected) = expected else { + return Ok(()); + }; + let manifest_sha = manifest.git_sha().as_str(); + if manifest_sha.len() != 40 || manifest_sha != expected.as_str() { + return Err(PrebuiltError::GitShaMismatch { + manifest: manifest_sha.to_owned(), + expected: expected.to_string(), + }); + } + Ok(()) +} /// Rename extracted files into the staged `lib@.` format. fn apply_staging_filenames( extracted_files: &[String], diff --git a/installer/src/prebuilt_provenance_tests.rs b/installer/src/prebuilt_provenance_tests.rs new file mode 100644 index 00000000..75a8c47f --- /dev/null +++ b/installer/src/prebuilt_provenance_tests.rs @@ -0,0 +1,36 @@ +//! Regression coverage for pinned prebuilt provenance gating. + +use super::*; +use crate::artefact::download::MockArtefactDownloader; +use crate::artefact::extraction::MockArtefactExtractor; +use crate::git::CommitSha; +use crate::test_utils::sha256_hex; + +#[test] +fn mismatched_pinned_manifest_skips_archive_download() { + let (_temp, destination_dir) = destination_dir(); + let expected = CommitSha::try_from(MATCHING_COMMIT).expect("full test commit SHA"); + let config = PrebuiltConfig { + expected_git_sha: Some(&expected), + ..base_config(&destination_dir) + }; + let fake_sha = sha256_hex(FAKE_ARCHIVE); + let manifest = + manifest_with_git_sha(MISMATCHED_COMMIT, &fake_sha).expect("construct mismatched manifest"); + let manifest_json = serde_json::to_string(&manifest).expect("serialize mismatched manifest"); + let mut downloader = MockArtefactDownloader::new(); + downloader + .expect_download_manifest() + .returning(move |_| Ok(manifest_json.clone())); + downloader.expect_download_archive().times(0); + let mut extractor = MockArtefactExtractor::new(); + extractor.expect_extract().times(0); + + let mut stderr = Vec::new(); + let result = attempt_prebuilt_with(&config, &downloader, &extractor, &mut stderr); + + assert!( + matches!(result, PrebuiltResult::Fallback { ref reason } if reason.contains("SHA mismatch")), + "a mismatched pinned manifest must fail before archive download, got {result:?}" + ); +} diff --git a/installer/src/prebuilt_tests.rs b/installer/src/prebuilt_tests.rs index 7dd75a04..e9d53f39 100644 --- a/installer/src/prebuilt_tests.rs +++ b/installer/src/prebuilt_tests.rs @@ -3,19 +3,205 @@ use super::*; use crate::artefact::download::MockArtefactDownloader; use crate::artefact::extraction::MockArtefactExtractor; +use crate::git::CommitSha; use crate::test_utils::{prebuilt_manifest_json, sha256_hex}; +use cap_std::{ambient_authority, fs::Dir}; +use proptest::prelude::*; +use proptest::test_runner::TestCaseError; use rstest::rstest; +use std::path::Path; const FAKE_ARCHIVE: &[u8] = b"fake archive content"; const TARGET: &str = "x86_64-unknown-linux-gnu"; const TOOLCHAIN: &str = "nightly-2026-05-28"; +/// A full 40-hex commit SHA beginning with the test manifest's `abc1234`. +const MATCHING_COMMIT: &str = "abc12340000000000000000000000000000000ab"; + +/// A full 40-hex commit SHA that does not share the manifest's prefix. +const MISMATCHED_COMMIT: &str = "deadbeef00000000000000000000000000000000"; + +fn commit_sha_strategy() -> impl Strategy { + const HEX_DIGITS: &[u8; 16] = b"0123456789abcdef"; + prop::collection::vec(0_u8..16, 40).prop_map(|nibbles| { + nibbles + .into_iter() + .map(|nibble| char::from(HEX_DIGITS[usize::from(nibble)])) + .collect() + }) +} + +fn manifest_with_git_sha(git_sha: &str, sha256: &str) -> serde_json::Result { + serde_json::from_value(serde_json::json!({ + "git_sha": git_sha, + "schema_version": 1, + "toolchain": TOOLCHAIN, + "target": TARGET, + "generated_at": "2026-02-03T00:00:00Z", + "files": ["libwhitaker_suite.so"], + "sha256": sha256, + })) +} + +/// Writes a test file relative to a capability rooted at its parent directory. +fn write_test_file(path: &Path, contents: &[u8]) -> std::io::Result<()> { + let parent = path.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "test file path has no parent directory", + ) + })?; + let file_name = path.file_name().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "test file path has no file name", + ) + })?; + let dir = Dir::open_ambient_dir(parent, ambient_authority())?; + dir.write(file_name, contents) +} + +proptest! { + #[test] + fn equal_full_git_shas_are_accepted( + commit in commit_sha_strategy(), + ) { + let expected = CommitSha::try_from(commit.as_str()) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let manifest = manifest_with_git_sha(&commit, &"a".repeat(64)) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + + prop_assert!(validate_git_sha(&manifest, Some(&expected)).is_ok()); + } + + #[test] + fn abbreviated_manifest_git_shas_are_rejected_for_pinned_installs( + commit in commit_sha_strategy(), + prefix_len in 7_usize..40, + ) { + let expected = CommitSha::try_from(commit.as_str()) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let manifest_sha = &commit[..prefix_len]; + let manifest = manifest_with_git_sha(manifest_sha, &"a".repeat(64)) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + + match validate_git_sha(&manifest, Some(&expected)) { + Err(PrebuiltError::GitShaMismatch { manifest, expected }) => { + prop_assert_eq!(manifest, manifest_sha); + prop_assert_eq!(expected, commit); + } + other => { + return Err(TestCaseError::fail(format!( + "expected GitShaMismatch, got {other:?}" + ))); + } + } + } + + #[test] + fn distinct_full_git_shas_sharing_a_prefix_are_rejected( + commit in commit_sha_strategy(), + shared_prefix_len in 7_usize..40, + ) { + let expected = CommitSha::try_from(commit.as_str()) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + let mut manifest_sha = commit.clone(); + let replacement = if manifest_sha.as_bytes()[shared_prefix_len] == b'0' { + "1" + } else { + "0" + }; + manifest_sha.replace_range(shared_prefix_len..=shared_prefix_len, replacement); + let manifest = manifest_with_git_sha(&manifest_sha, &"a".repeat(64)) + .map_err(|error| TestCaseError::fail(error.to_string()))?; + + match validate_git_sha(&manifest, Some(&expected)) { + Err(PrebuiltError::GitShaMismatch { manifest, expected }) => { + prop_assert_eq!(manifest, manifest_sha); + prop_assert_eq!(expected, commit); + } + other => { + return Err(TestCaseError::fail(format!( + "expected GitShaMismatch, got {other:?}" + ))); + } + } + } +} + fn base_config(destination_dir: &Utf8Path) -> PrebuiltConfig<'_> { PrebuiltConfig { target: TARGET, toolchain: TOOLCHAIN, destination_dir, quiet: true, + expected_git_sha: None, + } +} + +/// Construct downloader and extractor mocks for the successful prebuilt path. +fn success_mocks() -> (MockArtefactDownloader, MockArtefactExtractor) { + success_mocks_with_git_sha("abc1234") +} + +/// Construct successful mocks with the supplied manifest provenance. +fn success_mocks_with_git_sha(git_sha: &str) -> (MockArtefactDownloader, MockArtefactExtractor) { + let fake_sha = sha256_hex(FAKE_ARCHIVE); + let manifest = + manifest_with_git_sha(git_sha, &fake_sha).expect("construct manifest with Git SHA"); + let manifest_json = serde_json::to_string(&manifest).expect("serialize manifest with Git SHA"); + let mut downloader = MockArtefactDownloader::new(); + downloader + .expect_download_manifest() + .returning(move |_| Ok(manifest_json.clone())); + downloader + .expect_download_archive() + .returning(|_filename, dest| { + write_test_file(dest, FAKE_ARCHIVE).map_err(DownloadError::Io) + }); + let mut extractor = MockArtefactExtractor::new(); + extractor.expect_extract().returning(|_archive, dest| { + let source_name = "libwhitaker_suite.so".to_owned(); + write_test_file(&dest.join(&source_name), b"fake")?; + Ok(vec![source_name]) + }); + (downloader, extractor) +} + +#[rstest] +#[case::mismatch(Some(MISMATCHED_COMMIT), "abc1234", false)] +#[case::matching_full_sha(Some(MATCHING_COMMIT), MATCHING_COMMIT, true)] +#[case::unpinned_abbreviated_sha(None, "abc1234", true)] +fn prebuilt_git_sha_gate( + #[case] expected_git_sha: Option<&str>, + #[case] manifest_git_sha: &str, + #[case] expects_success: bool, +) { + let (_temp, destination_dir) = destination_dir(); + let expected_commit = expected_git_sha + .map(CommitSha::try_from) + .transpose() + .expect("full test commit SHA"); + let config = PrebuiltConfig { + expected_git_sha: expected_commit.as_ref(), + ..base_config(&destination_dir) + }; + let (downloader, extractor) = success_mocks_with_git_sha(manifest_git_sha); + let mut stderr = Vec::new(); + let result = attempt_prebuilt_with(&config, &downloader, &extractor, &mut stderr); + + if expects_success { + assert!( + matches!(result, PrebuiltResult::Success { .. }), + "expected Success, got {result:?}" + ); + } else { + match result { + PrebuiltResult::Fallback { reason } => { + assert!(reason.contains("SHA mismatch"), "reason: {reason}"); + } + other => panic!("expected Fallback, got {other:?}"), + } } } @@ -57,23 +243,7 @@ fn test_fallback_scenario( fn happy_path_returns_success() { let (_temp, destination_dir) = destination_dir(); let config = base_config(&destination_dir); - let fake_sha = sha256_hex(FAKE_ARCHIVE); - let manifest_json = prebuilt_manifest_json(TOOLCHAIN, TARGET, &fake_sha); - - let mut downloader = MockArtefactDownloader::new(); - downloader - .expect_download_manifest() - .returning(move |_| Ok(manifest_json.clone())); - downloader - .expect_download_archive() - .returning(|_filename, dest| std::fs::write(dest, FAKE_ARCHIVE).map_err(DownloadError::Io)); - - let mut extractor = MockArtefactExtractor::new(); - extractor.expect_extract().returning(|_archive, dest| { - let source_name = "libwhitaker_suite.so".to_owned(); - std::fs::write(dest.join(&source_name), b"fake").expect("write extracted file"); - Ok(vec![source_name]) - }); + let (downloader, extractor) = success_mocks(); let mut stderr = Vec::new(); let result = attempt_prebuilt_with(&config, &downloader, &extractor, &mut stderr); @@ -216,3 +386,6 @@ fn destination_creation_failure_returns_fallback() { other => panic!("expected Fallback, got {other:?}"), } } + +#[path = "prebuilt_provenance_tests.rs"] +mod prebuilt_provenance_tests; diff --git a/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_default_format.snap b/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_default_format.snap new file mode 100644 index 00000000..c91711b5 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_default_format.snap @@ -0,0 +1,17 @@ +--- +source: installer/src/output.rs +expression: info.display_text() +--- +Dry run - no files will be modified + +Workspace root: /home/user/whitaker +Toolchain: nightly-2025-01-15 +Target directory: /home/user/.local/share/dylint/lib +Verbosity level: 0 +Quiet: false +Skip deps: false +Skip wrapper: false +No update: false + +Crates to build: + - whitaker_suite diff --git a/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_pinned_format.snap b/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_pinned_format.snap new file mode 100644 index 00000000..4f762e60 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__output__tests__dry_run_display_matches_pinned_format.snap @@ -0,0 +1,19 @@ +--- +source: installer/src/output.rs +expression: info.display_text() +--- +Dry run - no files will be modified + +Workspace root: /home/user/whitaker +Toolchain: nightly-2025-01-15 +Target directory: /home/user/.local/share/dylint/lib +Verbosity level: 0 +Quiet: false +Skip deps: false +Skip wrapper: false +No update: false +Pinned ref: v0.2.5 +Parallel jobs: 4 + +Crates to build: + - whitaker_suite diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_is_silent_in_quiet_mode.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_is_silent_in_quiet_mode.snap new file mode 100644 index 00000000..2d1ab0dd --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_is_silent_in_quiet_mode.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "format!(\"{output:?}\")" +--- +"" diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_commit_fallback.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_commit_fallback.snap new file mode 100644 index 00000000..44335043 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_commit_fallback.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "String::from_utf8(output).expect(\"UTF-8 output\")" +--- +Pinned Whitaker suite to abc1234567890000000000000000000000000000 (abc123456789). diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_requested_ref.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_requested_ref.snap new file mode 100644 index 00000000..2911f238 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__pinned_checkout_reports_requested_ref.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "String::from_utf8(output).expect(\"UTF-8 output\")" +--- +Pinned Whitaker suite to v0.2.5 (abc123456789). diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_is_silent_in_quiet_mode.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_is_silent_in_quiet_mode.snap new file mode 100644 index 00000000..2d1ab0dd --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_is_silent_in_quiet_mode.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "format!(\"{output:?}\")" +--- +"" diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_clone_message.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_clone_message.snap new file mode 100644 index 00000000..a9669983 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_clone_message.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "String::from_utf8(output).expect(\"UTF-8 output\")" +--- +Cloning Whitaker repository to /managed/whitaker... diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_requested_pin_message.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_requested_pin_message.snap new file mode 100644 index 00000000..b480a2a1 --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_requested_pin_message.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "String::from_utf8(output).expect(\"UTF-8 output\")" +--- +Pinning Whitaker suite to v0.2.5... diff --git a/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_update_message.snap b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_update_message.snap new file mode 100644 index 00000000..a24d50af --- /dev/null +++ b/installer/src/snapshots/whitaker_installer__workspace_progress__tests__workspace_progress_reports_update_message.snap @@ -0,0 +1,5 @@ +--- +source: installer/src/workspace_progress.rs +expression: "String::from_utf8(output).expect(\"UTF-8 output\")" +--- +Updating Whitaker repository at /managed/whitaker... diff --git a/installer/src/tests.rs b/installer/src/tests.rs index 524be606..6600e863 100644 --- a/installer/src/tests.rs +++ b/installer/src/tests.rs @@ -224,6 +224,26 @@ fn ensure_dylint_tools_propagates_install_failures(test_base_dirs: TestBaseDirs) }); } +#[test] +fn unsupported_ref_is_rejected_before_dependency_install() { + let args = InstallArgs { + git_ref: Some("v0.2.5".to_owned()), + ..InstallArgs::default() + }; + let mut stderr = Vec::new(); + let mut installer_was_called = false; + + let action = WorkspaceAction::UseCurrentDir(Utf8PathBuf::from("/workspace/whitaker")); + let error = ensure_dependencies_after_ref_validation(&args, &action, &mut stderr, |_, _| { + installer_was_called = true; + Ok(()) + }) + .expect_err("current workspace must reject a pinned ref"); + + assert!(matches!(error, InstallerError::RefUnsupported { .. })); + assert!(!installer_was_called); +} + #[derive(Debug, Clone)] struct TestBaseDirs { home_dir: Option, diff --git a/installer/src/tests/fast_path.rs b/installer/src/tests/fast_path.rs index 4dd9aa08..d40b0f16 100644 --- a/installer/src/tests/fast_path.rs +++ b/installer/src/tests/fast_path.rs @@ -24,6 +24,7 @@ impl FastPathFixture { requested_crates: &self.requested_crates, toolchain: &self.toolchain, target_dir: &self.target_dir, + expected_git_sha: None, } } } diff --git a/installer/src/workspace.rs b/installer/src/workspace.rs index 3d06bbf5..a04495ac 100644 --- a/installer/src/workspace.rs +++ b/installer/src/workspace.rs @@ -1,14 +1,21 @@ -//! Workspace detection and path resolution. +//! Managed Whitaker workspace selection and preparation. //! -//! This module provides utilities for detecting whether the current directory -//! is a Whitaker workspace and for resolving platform-specific clone locations. +//! This module detects local workspaces, selects the managed-clone action, and +//! orchestrates clone, update, pinning, and detached-checkout recovery through +//! [`crate::git`]. It returns checkout provenance for prebuilt validation while +//! the CLI presentation boundary reports the completed action. use crate::dirs::BaseDirs; use crate::error::{InstallerError, Result}; +use crate::git::CommitSha; +use crate::workspace_lock::ManagedCloneLock; use camino::{Utf8Path, Utf8PathBuf}; +use tracing::debug; /// Repository URL for cloning Whitaker. -pub const WHITAKER_REPO_URL: &str = "https://github.com/leynos/whitaker"; +/// +/// Re-exported from [`crate::git`] to preserve the existing public path. +pub use crate::git::WHITAKER_REPO_URL; /// Expected package name in Cargo.toml to identify a Whitaker workspace. const WHITAKER_PACKAGE_NAME: &str = "whitaker"; @@ -74,6 +81,23 @@ pub enum WorkspaceAction { UseExisting(Utf8PathBuf), } +/// Selects the action needed to establish the Whitaker workspace. +/// +/// This is the shared, side-effect-free decision boundary used by real and +/// dry-run installation paths. +/// +/// # Errors +/// +/// Returns an error when the current directory or managed clone directory +/// cannot be determined. +pub fn resolve_workspace_action(dirs: &dyn BaseDirs, update: bool) -> Result { + let cwd = current_dir_utf8()?; + let clone_dir = clone_directory(dirs).ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: "could not determine data directory for cloning".to_owned(), + })?; + Ok(decide_workspace_action(&cwd, &clone_dir, update)) +} + /// Determines what action is needed to establish a Whitaker workspace. /// /// Examines the current directory and clone directory state to decide what @@ -98,49 +122,226 @@ pub fn decide_workspace_action( } } +/// The prepared workspace and, when a ref was pinned, its resolved commit. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WorkspaceCheckout { + /// Path to the workspace root the install should build from. + pub root: Utf8PathBuf, + /// The full commit SHA a `--ref` pin resolved to, if any. + pub pinned_commit: Option, + /// The existing detached HEAD reused by an unpinned `--no-update` install. + pub detached_commit: Option, + /// The action selected to prepare this workspace. + pub action: WorkspaceAction, +} + +/// Git operations owned by managed-workspace preparation. +/// +/// This private seam keeps production calls in this module while allowing the +/// lock behaviour to be tested without a network clone. +trait WorkspaceRepository { + fn clone(&self, target: &Utf8Path) -> Result<()>; + fn update(&self, repo: &Utf8Path) -> Result<()>; + fn ensure_default_branch(&self, repo: &Utf8Path) -> Result<()>; +} + +/// Production Git operations for managed-workspace preparation. +struct GitWorkspaceRepository; + +impl WorkspaceRepository for GitWorkspaceRepository { + fn clone(&self, target: &Utf8Path) -> Result<()> { + crate::git::clone_repository(target) + } + + fn update(&self, repo: &Utf8Path) -> Result<()> { + crate::git::update_repository(repo) + } + + fn ensure_default_branch(&self, repo: &Utf8Path) -> Result<()> { + crate::git::ensure_default_branch(repo) + } +} +/// Inputs shared by workspace preparation and its test boundary. +struct WorkspacePreparation<'a> { + dirs: &'a dyn BaseDirs, + update: bool, + git_ref: Option<&'a str>, +} + /// Ensures a Whitaker workspace is available, cloning if necessary. /// /// If the current directory is already a Whitaker workspace, returns its path. /// Otherwise, clones or updates the repository in the platform-specific data /// directory. Set `update` to `true` to run `git pull` on existing clones. /// +/// When `git_ref` is `Some`, the managed clone is pinned to that commit-ish +/// (SHA, tag, or branch): the ref is fetched first and checked out as a +/// detached HEAD, falling back to a locally resolvable ref or SHA if fetching +/// fails. Pinning is refused when the current directory is itself a Whitaker +/// workspace, since that would mutate the user's own working tree. The update +/// path first reattaches a detached clone to its default branch, so a previous +/// pin never breaks a later un-pinned install. +/// /// # Errors /// -/// Returns an error if the clone directory cannot be determined or if -/// cloning/updating fails. -pub fn ensure_workspace(dirs: &dyn BaseDirs, update: bool) -> Result { +/// Returns an error if the clone directory cannot be determined, if +/// cloning/updating fails, or if `git_ref` is requested for the current +/// directory workspace. +pub fn ensure_workspace( + dirs: &dyn BaseDirs, + update: bool, + git_ref: Option<&str>, +) -> Result { let cwd = current_dir_utf8()?; - let clone_dir = clone_directory(dirs).ok_or_else(|| InstallerError::WorkspaceNotFound { - reason: "could not determine data directory for cloning".to_owned(), - })?; + let preparation = WorkspacePreparation { + dirs, + update, + git_ref, + }; + ensure_workspace_from(&cwd, &preparation, &GitWorkspaceRepository) +} + +/// Prepares a workspace from an explicit current directory and Git boundary. +fn ensure_workspace_from( + cwd: &Utf8Path, + preparation: &WorkspacePreparation<'_>, + repository: &impl WorkspaceRepository, +) -> Result { + let clone_dir = + clone_directory(preparation.dirs).ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: "could not determine data directory for cloning".to_owned(), + })?; + if is_whitaker_workspace(cwd) { + let action = WorkspaceAction::UseCurrentDir(cwd.to_owned()); + ensure_ref_allowed(&action, preparation.git_ref)?; + return finalize_workspace_checkout(cwd.to_owned(), preparation.git_ref, action); + } - match decide_workspace_action(&cwd, &clone_dir, update) { - WorkspaceAction::UseCurrentDir(dir) | WorkspaceAction::UseExisting(dir) => Ok(dir), + let _lock = ManagedCloneLock::acquire(&clone_dir)?; + let action = decide_workspace_action(cwd, &clone_dir, preparation.update); + ensure_ref_allowed(&action, preparation.git_ref)?; + + let root = match &action { + WorkspaceAction::UseCurrentDir(dir) | WorkspaceAction::UseExisting(dir) => { + // UseCurrentDir is guaranteed refless by `ensure_ref_allowed`; + // UseExisting pins without pulling, per the `--no-update` contract. + dir.clone() + } WorkspaceAction::CloneTo(dir) => { - crate::git::clone_repository(&dir)?; - Ok(dir) + repository.clone(dir)?; + dir.clone() } WorkspaceAction::UpdateAt(dir) => { - crate::git::update_repository(&dir)?; - Ok(dir) + // Reattach before pulling so a prior detached pin cannot break the + // update, even when no new ref is requested. + repository.ensure_default_branch(dir)?; + repository.update(dir)?; + dir.clone() } + }; + finalize_workspace_checkout(root, preparation.git_ref, action) +} + +/// Applies an optional pin and constructs the resulting workspace checkout. +/// +/// This helper only owns the common tail after workspace setup. Cloning, +/// updating, and reattachment remain responsibilities of [`ensure_workspace`]. +pub(super) fn finalize_workspace_checkout( + root: Utf8PathBuf, + git_ref: Option<&str>, + action: WorkspaceAction, +) -> Result { + let pinned_commit = pin_if_requested(&root, git_ref)?; + let detached_commit = inherited_detached_commit(&root, git_ref, &action)?; + Ok(WorkspaceCheckout { + root, + pinned_commit, + detached_commit, + action, + }) +} + +/// Detects a detached commit inherited by an unpinned no-update checkout. +fn inherited_detached_commit( + root: &Utf8Path, + git_ref: Option<&str>, + action: &WorkspaceAction, +) -> Result> { + if git_ref.is_none() && matches!(action, WorkspaceAction::UseExisting(_)) { + return crate::git::detached_head_commit(root); + } + Ok(None) +} + +impl WorkspaceCheckout { + /// Returns the commit that a downloaded prebuilt artefact must match. + #[must_use] + pub fn expected_git_sha(&self) -> Option<&CommitSha> { + self.pinned_commit + .as_ref() + .or(self.detached_commit.as_ref()) + } +} + +/// Refuses `--ref` when the current directory is itself a Whitaker workspace. +/// +/// Pinning checks out a commit; doing so in the user's own working tree could +/// destroy uncommitted work, so it is rejected rather than attempted. +/// +/// # Errors +/// +/// Returns [`InstallerError::RefUnsupported`] when `action` uses the current +/// workspace and `git_ref` requests a pin. +pub fn ensure_ref_allowed(action: &WorkspaceAction, git_ref: Option<&str>) -> Result<()> { + if let (WorkspaceAction::UseCurrentDir(_), Some(git_ref)) = (action, git_ref) { + return Err(InstallerError::RefUnsupported { + git_ref: git_ref.to_owned(), + }); + } + Ok(()) +} + +/// Pins the managed clone to `git_ref` when one is requested. +/// +/// Returns the resolved commit SHA, or `None` when no ref was requested. +fn pin_if_requested(repo: &Utf8Path, git_ref: Option<&str>) -> Result> { + match git_ref { + Some(git_ref) => Ok(Some(pin_to_ref(repo, git_ref)?)), + None => Ok(None), } } +/// Fetches and checks out `git_ref`, falling back to local resolution offline. +pub(super) fn pin_to_ref(repo: &Utf8Path, git_ref: &str) -> Result { + let commit = match crate::git::fetch_ref(repo, git_ref) { + Ok(commit) => commit, + Err(fetch_error) => { + debug!( + operation = "resolve", + refspec = git_ref, + attempt = "local_fallback", + error = %fetch_error, + "remote pin fetch failed; resolving locally" + ); + crate::git::resolve_commit(repo, git_ref).map_err(|_| fetch_error)? + } + }; + crate::git::checkout_detached(repo, &commit)?; + Ok(commit) +} + /// Returns the workspace path without performing any side effects. /// /// If the current directory is a Whitaker workspace, returns it. Otherwise /// returns the platform-specific clone directory (which may not exist yet). /// Useful for dry-run mode to show what would happen without cloning. pub fn resolve_workspace_path(dirs: &dyn BaseDirs) -> Result { - let cwd = current_dir_utf8()?; - - if is_whitaker_workspace(&cwd) { - return Ok(cwd); - } - - clone_directory(dirs).ok_or_else(|| InstallerError::WorkspaceNotFound { - reason: "could not determine data directory for cloning".to_owned(), + let action = resolve_workspace_action(dirs, false)?; + Ok(match action { + WorkspaceAction::UseCurrentDir(dir) + | WorkspaceAction::CloneTo(dir) + | WorkspaceAction::UpdateAt(dir) + | WorkspaceAction::UseExisting(dir) => dir, }) } @@ -195,192 +396,5 @@ fn is_cargo_workspace_root(cargo_toml: &Utf8Path) -> Result { } #[cfg(test)] -mod tests { - use super::*; - use crate::dirs::{MockBaseDirs, SystemBaseDirs}; - use rstest::{fixture, rstest}; - use std::fs; - use std::path::PathBuf; - use tempfile::TempDir; - - /// A temporary directory converted to a UTF-8 path for workspace tests. - struct TempWorkspace { - _temp: TempDir, - path: Utf8PathBuf, - } - - #[fixture] - fn temp_workspace() -> TempWorkspace { - let temp = TempDir::new().expect("failed to create temp dir"); - let path = Utf8PathBuf::try_from(temp.path().to_owned()).expect("non-UTF8 temp path"); - TempWorkspace { _temp: temp, path } - } - - fn write_cargo_toml(dir: &Utf8Path, package_name: &str) { - let cargo_toml = dir.join("Cargo.toml"); - fs::write( - cargo_toml, - format!("[package]\nname = \"{package_name}\"\nversion = \"0.1.0\"\n"), - ) - .expect("failed to write Cargo.toml"); - } - - #[rstest] - #[case::whitaker_project(Some("whitaker"), true)] - #[case::other_project(Some("other-project"), false)] - #[case::empty_dir(None, false)] - fn is_whitaker_workspace_detection( - temp_workspace: TempWorkspace, - #[case] package_name: Option<&str>, - #[case] expected: bool, - ) { - if let Some(name) = package_name { - write_cargo_toml(&temp_workspace.path, name); - } - assert_eq!(is_whitaker_workspace(&temp_workspace.path), expected); - } - - #[test] - fn clone_directory_returns_some_on_supported_platforms() { - // This test may fail on unsupported platforms, but should pass on - // Linux, macOS, and Windows. - let dirs = SystemBaseDirs::new().expect("failed to create SystemBaseDirs"); - let dir = clone_directory(&dirs); - assert!(dir.is_some(), "expected clone_directory to return Some"); - assert!( - dir.as_ref() - .is_some_and(|p| p.as_str().contains("whitaker")), - "expected path to contain 'whitaker'" - ); - } - - #[rstest] - fn decide_workspace_action_uses_cwd_when_whitaker(temp_workspace: TempWorkspace) { - write_cargo_toml(&temp_workspace.path, "whitaker"); - let clone_dir = Utf8PathBuf::from("/nonexistent/clone/dir"); - - let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); - - assert_eq!(action, WorkspaceAction::UseCurrentDir(temp_workspace.path)); - } - - #[rstest] - fn decide_workspace_action_clones_when_empty(temp_workspace: TempWorkspace) { - // temp_workspace.path is empty (no Cargo.toml), clone_dir doesn't exist - let clone_dir = temp_workspace.path.join("clone_target"); - - let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); - - assert_eq!(action, WorkspaceAction::CloneTo(clone_dir)); - } - - #[rstest] - fn decide_workspace_action_updates_when_clone_exists(temp_workspace: TempWorkspace) { - // Create a clone directory (not a whitaker workspace, just exists) - let clone_dir = temp_workspace.path.join("clone_target"); - fs::create_dir(&clone_dir).expect("failed to create clone dir"); - - let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); - - assert_eq!(action, WorkspaceAction::UpdateAt(clone_dir)); - } - - #[rstest] - fn decide_workspace_action_uses_existing_when_no_update(temp_workspace: TempWorkspace) { - let clone_dir = temp_workspace.path.join("clone_target"); - fs::create_dir(&clone_dir).expect("failed to create clone dir"); - - let action = decide_workspace_action(&temp_workspace.path, &clone_dir, false); - - assert_eq!(action, WorkspaceAction::UseExisting(clone_dir)); - } - - // ------------------------------------------------------------------------- - // Behavioural tests for workspace orchestration with mocked dependencies - // ------------------------------------------------------------------------- - - fn mock_dirs_returning(data_dir: Option) -> MockBaseDirs { - let mut mock = MockBaseDirs::new(); - mock.expect_whitaker_data_dir().return_const(data_dir); - mock - } - - #[rstest] - fn resolve_workspace_path_returns_clone_dir_when_not_in_workspace( - temp_workspace: TempWorkspace, - ) { - // Mock returns a data directory inside temp workspace - let expected_dir = temp_workspace.path.join("data").join("whitaker"); - let mock = mock_dirs_returning(Some(expected_dir.clone().into_std_path_buf())); - - let result = resolve_workspace_path(&mock); - - assert!(result.is_ok()); - assert_eq!(result.unwrap(), expected_dir); - } - - #[rstest] - fn resolve_workspace_path_errors_when_data_dir_unavailable(temp_workspace: TempWorkspace) { - let _ = temp_workspace; // Ensure fixture is used - let mock = mock_dirs_returning(None); - - let result = resolve_workspace_path(&mock); - - assert!(result.is_err()); - let err = result.unwrap_err(); - assert!( - matches!(err, InstallerError::WorkspaceNotFound { .. }), - "expected WorkspaceNotFound error, got: {err:?}" - ); - } - - #[test] - fn clone_directory_returns_none_when_data_dir_unavailable() { - let mock = mock_dirs_returning(None); - assert!(clone_directory(&mock).is_none()); - } - - #[rstest] - fn clone_directory_returns_path_from_mock(temp_workspace: TempWorkspace) { - let expected = temp_workspace.path.join("data").join("whitaker"); - let mock = mock_dirs_returning(Some(expected.clone().into_std_path_buf())); - assert_eq!(clone_directory(&mock), Some(expected)); - } - - // Tests for find_workspace_root - - fn write_workspace_cargo_toml(dir: &Utf8Path) { - fs::write( - dir.join("Cargo.toml"), - "[workspace]\nmembers = [\"crates/*\"]\n", - ) - .expect("failed to write workspace Cargo.toml"); - } - - #[rstest] - fn find_workspace_root_finds_workspace_in_current_dir(temp_workspace: TempWorkspace) { - write_workspace_cargo_toml(&temp_workspace.path); - assert_eq!( - find_workspace_root(&temp_workspace.path).unwrap(), - temp_workspace.path - ); - } - - #[rstest] - fn find_workspace_root_finds_workspace_in_parent_dir(temp_workspace: TempWorkspace) { - write_workspace_cargo_toml(&temp_workspace.path); - let subdir = temp_workspace.path.join("crates").join("my_crate"); - fs::create_dir_all(&subdir).expect("failed to create subdirs"); - assert_eq!(find_workspace_root(&subdir).unwrap(), temp_workspace.path); - } - - #[rstest] - fn find_workspace_root_errors_when_no_workspace_found(temp_workspace: TempWorkspace) { - write_cargo_toml(&temp_workspace.path, "not_a_workspace"); - let result = find_workspace_root(&temp_workspace.path); - assert!(matches!( - result.unwrap_err(), - InstallerError::WorkspaceNotFound { .. } - )); - } -} +#[path = "workspace_tests.rs"] +mod tests; diff --git a/installer/src/workspace_lock.rs b/installer/src/workspace_lock.rs new file mode 100644 index 00000000..fb590455 --- /dev/null +++ b/installer/src/workspace_lock.rs @@ -0,0 +1,109 @@ +//! Cross-process serialization for managed Whitaker clone preparation. +//! +//! A sidecar lock prevents concurrent installers from changing the shared +//! managed checkout between action selection, ref resolution, and checkout. + +use crate::error::{InstallerError, Result}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ + ambient_authority, + fs_utf8::{Dir, OpenOptions}, +}; +use fs2::FileExt; +use std::fs::File; + +/// An exclusive advisory lock held while preparing the managed clone. +pub(crate) struct ManagedCloneLock { + _file: File, +} + +impl ManagedCloneLock { + /// Acquires the sidecar lock for `clone_dir`, waiting for another installer + /// to finish preparation before re-evaluating the workspace action. + pub(crate) fn acquire(clone_dir: &Utf8Path) -> Result { + let path = lock_path(clone_dir); + let parent = path + .parent() + .ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: format!("could not determine parent directory for workspace lock {path}"), + })?; + let file_name = path + .file_name() + .ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: format!("could not determine file name for workspace lock {path}"), + })?; + Dir::create_ambient_dir_all(parent, ambient_authority()).map_err(|source| { + InstallerError::WorkspaceLock { + path: path.clone(), + source, + } + })?; + let directory = Dir::open_ambient_dir(parent, ambient_authority()).map_err(|source| { + InstallerError::WorkspaceLock { + path: path.clone(), + source, + } + })?; + let mut options = OpenOptions::new(); + options.create(true).truncate(false).read(true).write(true); + let file = directory + .open_with(file_name, &options) + .map(|file| file.into_std()) + .map_err(|source| InstallerError::WorkspaceLock { + path: path.clone(), + source, + })?; + file.lock_exclusive() + .map_err(|source| InstallerError::WorkspaceLock { path, source })?; + Ok(Self { _file: file }) + } +} + +/// Returns the persistent sidecar lock path for a managed clone directory. +#[must_use] +pub(crate) fn lock_path(clone_dir: &Utf8Path) -> Utf8PathBuf { + clone_dir.with_extension("lock") +} + +#[cfg(test)] +mod tests { + //! Validates capability-scoped managed-clone lock access and exclusion. + + use super::ManagedCloneLock; + use camino::Utf8PathBuf; + use cap_std::{ + ambient_authority, + fs_utf8::{Dir, OpenOptions}, + }; + use fs2::FileExt; + use tempfile::TempDir; + + #[test] + fn managed_clone_lock_excludes_another_open_file() { + let temp = TempDir::new().expect("create temporary lock directory"); + let clone_dir = Utf8PathBuf::try_from(temp.path().join("whitaker")) + .expect("temporary lock path is UTF-8"); + let first = ManagedCloneLock::acquire(&clone_dir).expect("acquire first lock"); + let parent = clone_dir + .parent() + .expect("temporary clone directory has a parent"); + let directory = Dir::open_ambient_dir(parent, ambient_authority()) + .expect("open temporary lock directory capability"); + let mut options = OpenOptions::new(); + options.read(true).write(true); + let second = directory + .open_with("whitaker.lock", &options) + .expect("open second lock handle through directory capability") + .into_std(); + + let error = second + .try_lock_exclusive() + .expect_err("second handle must contend for the lock"); + assert_eq!(error.kind(), fs2::lock_contended_error().kind()); + + drop(first); + second + .try_lock_exclusive() + .expect("second handle acquires lock after release"); + } +} diff --git a/installer/src/workspace_lock_workflow_tests.rs b/installer/src/workspace_lock_workflow_tests.rs new file mode 100644 index 00000000..a60361db --- /dev/null +++ b/installer/src/workspace_lock_workflow_tests.rs @@ -0,0 +1,242 @@ +//! Real-Git workflow coverage for managed-clone lock serialization. + +use super::super::{ + WorkspaceAction, WorkspacePreparation, WorkspaceRepository, ensure_workspace_from, +}; +use crate::dirs::BaseDirs; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use std::path::PathBuf; +use std::process::Command; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex, mpsc}; +use std::thread; +use std::time::Duration; +use tempfile::TempDir; + +#[derive(Clone)] +struct ManagedCloneDirs { + clone_dir: PathBuf, +} + +impl BaseDirs for ManagedCloneDirs { + fn home_dir(&self) -> Option { + None + } + + fn bin_dir(&self) -> Option { + None + } + + fn whitaker_data_dir(&self) -> Option { + Some(self.clone_dir.clone()) + } +} + +fn git(dir: &Utf8Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(dir.as_std_path()) + .output() + .expect("spawn fixture Git command"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_owned() +} + +fn commit_file(dir: &Utf8Path, contents: &str, message: &str) -> String { + let directory = + Dir::open_ambient_dir(dir, ambient_authority()).expect("open source fixture capability"); + directory + .write("fixture.txt", contents) + .expect("write source fixture file"); + git(dir, &["add", "."]); + git( + dir, + &[ + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "-c", + "commit.gpgsign=false", + "commit", + "-m", + message, + ], + ); + git(dir, &["rev-parse", "HEAD"]) +} + +fn clone_repository(source: &Utf8Path, target: &Utf8Path) { + let output = Command::new("git") + .args(["clone", source.as_str(), target.as_str()]) + .output() + .expect("spawn fixture clone"); + assert!( + output.status.success(), + "fixture clone failed: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +struct BlockingGitWorkspaceRepository { + source: Utf8PathBuf, + clone_started: mpsc::Sender<()>, + release_clone: Mutex>, + clone_calls: AtomicUsize, + update_calls: AtomicUsize, + branch_repair_calls: AtomicUsize, +} + +impl WorkspaceRepository for BlockingGitWorkspaceRepository { + fn clone(&self, target: &Utf8Path) -> crate::error::Result<()> { + self.clone_calls.fetch_add(1, Ordering::SeqCst); + self.clone_started + .send(()) + .expect("report clone after acquiring managed-clone lock"); + self.release_clone + .lock() + .expect("lock clone-release receiver") + .recv() + .expect("release pinned clone"); + clone_repository(&self.source, target); + Ok(()) + } + + fn update(&self, repo: &Utf8Path) -> crate::error::Result<()> { + self.update_calls.fetch_add(1, Ordering::SeqCst); + crate::git::update_repository(repo) + } + + fn ensure_default_branch(&self, repo: &Utf8Path) -> crate::error::Result<()> { + self.branch_repair_calls.fetch_add(1, Ordering::SeqCst); + crate::git::ensure_default_branch(repo) + } +} + +#[test] +fn pinned_and_unpinned_preparations_serialize_and_recheck_state() { + let temp = TempDir::new().expect("create temporary workflow directory"); + let root = Utf8PathBuf::try_from(temp.path().to_owned()).expect("temporary path is UTF-8"); + let directory = Dir::open_ambient_dir(&root, ambient_authority()) + .expect("open temporary workflow capability"); + directory + .create_dir_all("source/caller/data") + .expect("create workflow fixture directories"); + let source = root.join("source"); + let caller = root.join("caller"); + let managed_clone = root.join("caller/data/whitaker"); + git(&source, &["init", "-b", "main"]); + let pinned_commit = commit_file(&source, "first", "initial commit"); + git(&source, &["tag", "v1"]); + let updated_commit = commit_file(&source, "second", "remote update"); + + let dirs = ManagedCloneDirs { + clone_dir: managed_clone.clone().into_std_path_buf(), + }; + let (clone_started_sender, clone_started_receiver) = mpsc::channel(); + let (release_clone_sender, release_clone_receiver) = mpsc::channel(); + let repository = Arc::new(BlockingGitWorkspaceRepository { + source: source.clone(), + clone_started: clone_started_sender, + release_clone: Mutex::new(release_clone_receiver), + clone_calls: AtomicUsize::new(0), + update_calls: AtomicUsize::new(0), + branch_repair_calls: AtomicUsize::new(0), + }); + + let pinned = { + let caller = caller.clone(); + let dirs = dirs.clone(); + let repository = Arc::clone(&repository); + thread::spawn(move || { + let preparation = WorkspacePreparation { + dirs: &dirs, + update: true, + git_ref: Some("v1"), + }; + ensure_workspace_from(&caller, &preparation, &*repository) + }) + }; + clone_started_receiver + .recv_timeout(Duration::from_secs(1)) + .expect("pinned preparation enters clone after acquiring the lock"); + + let (unpinned_started_sender, unpinned_started_receiver) = mpsc::channel(); + let (unpinned_result_sender, unpinned_result_receiver) = mpsc::channel(); + let unpinned = { + let caller = caller.clone(); + let dirs = dirs.clone(); + let repository = Arc::clone(&repository); + thread::spawn(move || { + unpinned_started_sender + .send(()) + .expect("report unpinned preparation start"); + let preparation = WorkspacePreparation { + dirs: &dirs, + update: true, + git_ref: None, + }; + unpinned_result_sender.send(ensure_workspace_from(&caller, &preparation, &*repository)) + }) + }; + unpinned_started_receiver + .recv_timeout(Duration::from_secs(1)) + .expect("unpinned preparation begins while the pinned clone holds the lock"); + assert!( + unpinned_result_receiver + .recv_timeout(Duration::from_millis(100)) + .is_err(), + "unpinned preparation must wait for the managed-clone lock" + ); + assert_eq!(repository.clone_calls.load(Ordering::SeqCst), 1); + assert_eq!(repository.update_calls.load(Ordering::SeqCst), 0); + assert_eq!(repository.branch_repair_calls.load(Ordering::SeqCst), 0); + + release_clone_sender.send(()).expect("release pinned clone"); + + let pinned = pinned + .join() + .expect("pinned preparation thread should not panic") + .expect("pinned preparation should succeed"); + let unpinned_checkout = unpinned_result_receiver + .recv_timeout(Duration::from_secs(5)) + .expect("unpinned preparation completes after lock release") + .expect("unpinned preparation should succeed"); + unpinned + .join() + .expect("unpinned preparation thread should not panic") + .expect("unpinned preparation result should be delivered"); + + assert_eq!( + pinned.action, + WorkspaceAction::CloneTo(managed_clone.clone()) + ); + assert_eq!( + pinned.pinned_commit.as_ref().map(|commit| commit.as_str()), + Some(pinned_commit.as_str()) + ); + assert_eq!( + unpinned_checkout.action, + WorkspaceAction::UpdateAt(managed_clone.clone()) + ); + assert_eq!(unpinned_checkout.pinned_commit, None); + assert_eq!( + crate::git::resolve_commit(&managed_clone, "refs/whitaker/pinned-ref") + .expect("fetch stores pinned ref") + .as_str(), + pinned_commit + ); + assert_eq!( + git(&managed_clone, &["symbolic-ref", "HEAD"]), + "refs/heads/main" + ); + assert_eq!(git(&managed_clone, &["rev-parse", "HEAD"]), updated_commit); + assert_eq!(repository.clone_calls.load(Ordering::SeqCst), 1); + assert_eq!(repository.update_calls.load(Ordering::SeqCst), 1); + assert_eq!(repository.branch_repair_calls.load(Ordering::SeqCst), 1); +} diff --git a/installer/src/workspace_progress.rs b/installer/src/workspace_progress.rs new file mode 100644 index 00000000..070d1547 --- /dev/null +++ b/installer/src/workspace_progress.rs @@ -0,0 +1,194 @@ +//! Operator-facing progress messages for managed workspace operations. +//! +//! This module keeps CLI reporting separate from checkout mutation. It reports +//! the action recorded by workspace preparation and the resolved pin. + +use std::io::Write; +use whitaker_installer::cli::InstallArgs; +use whitaker_installer::git::CommitSha; +use whitaker_installer::output::write_stderr_line; +use whitaker_installer::workspace::{WorkspaceAction, WorkspaceCheckout}; + +/// Reports the workspace action and requested pin selected during preparation. +pub(super) fn report_workspace_progress( + args: &InstallArgs, + checkout: &WorkspaceCheckout, + stderr: &mut dyn Write, +) { + if args.quiet { + return; + } + + match &checkout.action { + WorkspaceAction::CloneTo(dir) => { + write_stderr_line(stderr, format!("Cloning Whitaker repository to {dir}...")); + } + WorkspaceAction::UpdateAt(dir) => { + write_stderr_line(stderr, format!("Updating Whitaker repository at {dir}...")); + } + WorkspaceAction::UseCurrentDir(_) | WorkspaceAction::UseExisting(_) => {} + } + + if let Some(git_ref) = &args.git_ref { + write_stderr_line(stderr, format!("Pinning Whitaker suite to {git_ref}...")); + } +} + +/// Reports the resolved commit after a pinned checkout succeeds. +pub(super) fn report_pinned_checkout( + quiet: bool, + git_ref: Option<&str>, + checkout: &WorkspaceCheckout, + stderr: &mut dyn Write, +) { + if quiet { + return; + } + let Some(commit) = &checkout.pinned_commit else { + return; + }; + + write_stderr_line( + stderr, + format!( + "Pinned Whitaker suite to {} ({}).", + git_ref.unwrap_or(commit.as_str()), + short_commit(commit) + ), + ); +} + +/// Abbreviates a commit SHA to its leading 12 characters for display. +fn short_commit(commit: &CommitSha) -> &str { + let commit = commit.as_str(); + let end = commit.len().min(12); + &commit[..end] +} + +#[cfg(test)] +mod tests { + use super::*; + use camino::Utf8PathBuf; + use rstest::rstest; + + const COMMIT: &str = "abc1234567890000000000000000000000000000"; + + fn pinned_checkout() -> WorkspaceCheckout { + let root = Utf8PathBuf::from("/managed/whitaker"); + WorkspaceCheckout { + root: root.clone(), + pinned_commit: Some(CommitSha::try_from(COMMIT).expect("full test commit SHA")), + detached_commit: None, + action: WorkspaceAction::UseExisting(root), + } + } + + fn checkout(action: WorkspaceAction) -> WorkspaceCheckout { + WorkspaceCheckout { + root: Utf8PathBuf::from("/managed/whitaker"), + pinned_commit: None, + detached_commit: None, + action, + } + } + + #[rstest] + #[case::clone( + WorkspaceAction::CloneTo(Utf8PathBuf::from("/managed/whitaker")), + "workspace_progress_reports_clone_message" + )] + #[case::update( + WorkspaceAction::UpdateAt(Utf8PathBuf::from("/managed/whitaker")), + "workspace_progress_reports_update_message" + )] + fn workspace_progress_reports_exact_action_message( + #[case] action: WorkspaceAction, + #[case] snapshot_name: &str, + ) { + let mut output = Vec::new(); + + report_workspace_progress(&InstallArgs::default(), &checkout(action), &mut output); + + insta::assert_snapshot!( + snapshot_name, + String::from_utf8(output).expect("UTF-8 output") + ); + } + + #[test] + fn workspace_progress_reports_requested_pin_message() { + let args = InstallArgs { + git_ref: Some("v0.2.5".to_owned()), + ..InstallArgs::default() + }; + let mut output = Vec::new(); + + report_workspace_progress( + &args, + &checkout(WorkspaceAction::UseExisting(Utf8PathBuf::from( + "/managed/whitaker", + ))), + &mut output, + ); + + insta::assert_snapshot!( + "workspace_progress_reports_requested_pin_message", + String::from_utf8(output).expect("UTF-8 output") + ); + } + + #[test] + fn workspace_progress_is_silent_in_quiet_mode() { + let args = InstallArgs { + quiet: true, + ..InstallArgs::default() + }; + let mut output = Vec::new(); + + report_workspace_progress( + &args, + &checkout(WorkspaceAction::CloneTo(Utf8PathBuf::from( + "/managed/whitaker", + ))), + &mut output, + ); + + let output = String::from_utf8(output).expect("UTF-8 output"); + + insta::assert_snapshot!( + "workspace_progress_is_silent_in_quiet_mode", + format!("{output:?}") + ); + } + + #[rstest] + #[case::requested_ref(Some("v0.2.5"), "pinned_checkout_reports_requested_ref")] + #[case::commit_fallback(None, "pinned_checkout_reports_commit_fallback")] + fn pinned_checkout_reports_exact_message( + #[case] git_ref: Option<&str>, + #[case] snapshot_name: &str, + ) { + let mut output = Vec::new(); + + report_pinned_checkout(false, git_ref, &pinned_checkout(), &mut output); + + insta::assert_snapshot!( + snapshot_name, + String::from_utf8(output).expect("UTF-8 output") + ); + } + + #[test] + fn pinned_checkout_is_silent_in_quiet_mode() { + let mut output = Vec::new(); + + report_pinned_checkout(true, Some("v0.2.5"), &pinned_checkout(), &mut output); + + let output = String::from_utf8(output).expect("UTF-8 output"); + + insta::assert_snapshot!( + "pinned_checkout_is_silent_in_quiet_mode", + format!("{output:?}") + ); + } +} diff --git a/installer/src/workspace_tests.rs b/installer/src/workspace_tests.rs new file mode 100644 index 00000000..0e56ad6a --- /dev/null +++ b/installer/src/workspace_tests.rs @@ -0,0 +1,398 @@ +//! Tests for workspace detection and checkout orchestration. + +use super::*; +use crate::dirs::{MockBaseDirs, SystemBaseDirs}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use rstest::{fixture, rstest}; +use std::path::PathBuf; +use std::sync::{ + Arc, Barrier, Mutex, + atomic::{AtomicUsize, Ordering}, + mpsc, +}; +use std::thread; +use std::time::Duration; +use tempfile::TempDir; + +/// A temporary directory converted to a UTF-8 path for workspace tests. +struct TempWorkspace { + _temp: TempDir, + path: Utf8PathBuf, + dir: Dir, +} + +#[fixture] +fn temp_workspace() -> TempWorkspace { + let temp = TempDir::new().expect("failed to create temp dir"); + let path = Utf8PathBuf::try_from(temp.path().to_owned()).expect("non-UTF8 temp path"); + let dir = Dir::open_ambient_dir(&path, ambient_authority()) + .expect("failed to open temporary workspace directory"); + TempWorkspace { + _temp: temp, + path, + dir, + } +} + +fn write_cargo_toml(dir: &Dir, package_name: &str) { + dir.write( + "Cargo.toml", + format!("[package]\nname = \"{package_name}\"\nversion = \"0.1.0\"\n"), + ) + .expect("failed to write Cargo.toml"); +} + +#[rstest] +#[case::whitaker_project(Some("whitaker"), true)] +#[case::other_project(Some("other-project"), false)] +#[case::empty_dir(None, false)] +fn is_whitaker_workspace_detection( + temp_workspace: TempWorkspace, + #[case] package_name: Option<&str>, + #[case] expected: bool, +) { + if let Some(name) = package_name { + write_cargo_toml(&temp_workspace.dir, name); + } + assert_eq!(is_whitaker_workspace(&temp_workspace.path), expected); +} + +#[test] +fn clone_directory_returns_some_on_supported_platforms() { + // This test may fail on unsupported platforms, but should pass on + // Linux, macOS, and Windows. + let dirs = SystemBaseDirs::new().expect("failed to create SystemBaseDirs"); + let dir = clone_directory(&dirs); + assert!(dir.is_some(), "expected clone_directory to return Some"); + assert!( + dir.as_ref() + .is_some_and(|p| p.as_str().contains("whitaker")), + "expected path to contain 'whitaker'" + ); +} + +#[rstest] +fn decide_workspace_action_uses_cwd_when_whitaker(temp_workspace: TempWorkspace) { + write_cargo_toml(&temp_workspace.dir, "whitaker"); + let clone_dir = Utf8PathBuf::from("/nonexistent/clone/dir"); + + let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); + + assert_eq!(action, WorkspaceAction::UseCurrentDir(temp_workspace.path)); +} + +#[rstest] +fn decide_workspace_action_clones_when_empty(temp_workspace: TempWorkspace) { + // temp_workspace.path is empty (no Cargo.toml), clone_dir doesn't exist + let clone_dir = temp_workspace.path.join("clone_target"); + + let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); + + assert_eq!(action, WorkspaceAction::CloneTo(clone_dir)); +} + +#[rstest] +fn decide_workspace_action_updates_when_clone_exists(temp_workspace: TempWorkspace) { + // Create a clone directory (not a whitaker workspace, just exists) + let clone_dir = temp_workspace.path.join("clone_target"); + temp_workspace + .dir + .create_dir("clone_target") + .expect("failed to create clone dir"); + + let action = decide_workspace_action(&temp_workspace.path, &clone_dir, true); + + assert_eq!(action, WorkspaceAction::UpdateAt(clone_dir)); +} + +#[rstest] +fn decide_workspace_action_uses_existing_when_no_update(temp_workspace: TempWorkspace) { + let clone_dir = temp_workspace.path.join("clone_target"); + temp_workspace + .dir + .create_dir("clone_target") + .expect("failed to create clone dir"); + + let action = decide_workspace_action(&temp_workspace.path, &clone_dir, false); + + assert_eq!(action, WorkspaceAction::UseExisting(clone_dir)); +} + +#[test] +fn ensure_ref_allowed_refuses_current_dir_workspace() { + let action = WorkspaceAction::UseCurrentDir(Utf8PathBuf::from("/some/whitaker")); + let err = ensure_ref_allowed(&action, Some("v0.2.5")).expect_err("expected refusal"); + let InstallerError::RefUnsupported { git_ref } = &err else { + panic!("expected RefUnsupported, got {err:?}"); + }; + assert_eq!(git_ref, "v0.2.5"); + assert_eq!( + err.to_string(), + concat!( + "cannot pin --ref v0.2.5: the current directory is itself a Whitaker ", + "workspace; run the installer from outside a checkout to pin the suite" + ) + ); + + let InstallerError::RefUnsupported { git_ref } = err.clone() else { + panic!("cloned error changed variant"); + }; + assert_eq!(git_ref, "v0.2.5"); +} + +#[test] +fn ensure_ref_allowed_permits_current_dir_without_ref() { + let action = WorkspaceAction::UseCurrentDir(Utf8PathBuf::from("/some/whitaker")); + assert!(ensure_ref_allowed(&action, None).is_ok()); +} + +#[rstest] +#[case::clone(WorkspaceAction::CloneTo(Utf8PathBuf::from("/clone")))] +#[case::update(WorkspaceAction::UpdateAt(Utf8PathBuf::from("/clone")))] +#[case::existing(WorkspaceAction::UseExisting(Utf8PathBuf::from("/clone")))] +fn ensure_ref_allowed_permits_ref_for_managed_clones(#[case] action: WorkspaceAction) { + assert!(ensure_ref_allowed(&action, Some("v0.2.5")).is_ok()); +} + +// ------------------------------------------------------------------------- +// Behavioural tests for workspace orchestration with mocked dependencies +// ------------------------------------------------------------------------- + +fn mock_dirs_returning(data_dir: Option) -> MockBaseDirs { + let mut mock = MockBaseDirs::new(); + mock.expect_whitaker_data_dir().return_const(data_dir); + mock +} + +#[rstest] +fn resolve_workspace_path_returns_clone_dir_when_not_in_workspace(temp_workspace: TempWorkspace) { + // Mock returns a data directory inside temp workspace + let expected_dir = temp_workspace.path.join("data").join("whitaker"); + let mock = mock_dirs_returning(Some(expected_dir.clone().into_std_path_buf())); + + let result = resolve_workspace_path(&mock); + + assert!(result.is_ok()); + assert_eq!( + result.expect("workspace path should resolve from the mock data directory"), + expected_dir + ); +} + +#[test] +fn resolve_workspace_path_errors_when_data_dir_unavailable() { + let mock = mock_dirs_returning(None); + + let result = resolve_workspace_path(&mock); + + assert!(result.is_err()); + let err = result.expect_err("workspace path should fail without a data directory"); + assert!( + matches!(err, InstallerError::WorkspaceNotFound { .. }), + "expected WorkspaceNotFound error, got: {err:?}" + ); +} + +#[test] +fn clone_directory_returns_none_when_data_dir_unavailable() { + let mock = mock_dirs_returning(None); + assert!(clone_directory(&mock).is_none()); +} + +#[rstest] +fn clone_directory_returns_path_from_mock(temp_workspace: TempWorkspace) { + let expected = temp_workspace.path.join("data").join("whitaker"); + let mock = mock_dirs_returning(Some(expected.clone().into_std_path_buf())); + assert_eq!(clone_directory(&mock), Some(expected)); +} + +#[derive(Clone)] +struct FixedBaseDirs { + data_dir: PathBuf, +} + +impl BaseDirs for FixedBaseDirs { + fn home_dir(&self) -> Option { + None + } + + fn bin_dir(&self) -> Option { + None + } + + fn whitaker_data_dir(&self) -> Option { + Some(self.data_dir.clone()) + } +} + +struct ConcurrentWorkspaceRepository { + clone_started: mpsc::Sender<()>, + release_clone: Mutex>, + clone_calls: AtomicUsize, + update_calls: AtomicUsize, +} + +impl WorkspaceRepository for ConcurrentWorkspaceRepository { + fn clone(&self, target: &Utf8Path) -> Result<()> { + self.clone_calls.fetch_add(1, Ordering::SeqCst); + self.clone_started + .send(()) + .expect("report mock clone start"); + self.release_clone + .lock() + .expect("lock mock clone release receiver") + .recv() + .expect("release mock clone"); + let parent = target + .parent() + .ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: format!("mock clone target has no parent: {target}"), + })?; + let name = target + .file_name() + .ok_or_else(|| InstallerError::WorkspaceNotFound { + reason: format!("mock clone target has no file name: {target}"), + })?; + let directory = Dir::open_ambient_dir(parent, ambient_authority())?; + directory.create_dir(name)?; + Ok(()) + } + + fn update(&self, _repo: &Utf8Path) -> Result<()> { + self.update_calls.fetch_add(1, Ordering::SeqCst); + Ok(()) + } + + fn ensure_default_branch(&self, _repo: &Utf8Path) -> Result<()> { + Ok(()) + } +} + +#[rstest] +fn concurrent_workspace_preparation_waits_and_rechecks_action(temp_workspace: TempWorkspace) { + temp_workspace + .dir + .create_dir_all("caller/data") + .expect("create caller and data directories"); + let cwd = temp_workspace.path.join("caller"); + let clone_dir = temp_workspace.path.join("caller/data/whitaker"); + let dirs = FixedBaseDirs { + data_dir: clone_dir.clone().into_std_path_buf(), + }; + let (clone_started_sender, clone_started_receiver) = mpsc::channel(); + let (release_clone_sender, release_clone_receiver) = mpsc::channel(); + let repository = Arc::new(ConcurrentWorkspaceRepository { + clone_started: clone_started_sender, + release_clone: Mutex::new(release_clone_receiver), + clone_calls: AtomicUsize::new(0), + update_calls: AtomicUsize::new(0), + }); + + let first = { + let cwd = cwd.clone(); + let dirs = dirs.clone(); + let repository = Arc::clone(&repository); + thread::spawn(move || { + let preparation = WorkspacePreparation { + dirs: &dirs, + update: true, + git_ref: None, + }; + ensure_workspace_from(&cwd, &preparation, &*repository) + }) + }; + clone_started_receiver + .recv_timeout(Duration::from_secs(1)) + .expect("first preparation begins cloning while holding the lock"); + + let start_barrier = Arc::new(Barrier::new(2)); + let (second_ready_sender, second_ready_receiver) = mpsc::channel(); + let second = { + let cwd = cwd.clone(); + let dirs = dirs.clone(); + let repository = Arc::clone(&repository); + let start_barrier = Arc::clone(&start_barrier); + thread::spawn(move || { + second_ready_sender + .send(()) + .expect("report second preparation ready"); + start_barrier.wait(); + let preparation = WorkspacePreparation { + dirs: &dirs, + update: true, + git_ref: None, + }; + ensure_workspace_from(&cwd, &preparation, &*repository) + }) + }; + second_ready_receiver + .recv_timeout(Duration::from_secs(1)) + .expect("second preparation is ready to contend"); + start_barrier.wait(); + assert!( + clone_started_receiver + .recv_timeout(Duration::from_millis(100)) + .is_err(), + "second preparation must wait for the managed-clone lock" + ); + + release_clone_sender + .send(()) + .expect("release first preparation clone"); + let first = first + .join() + .expect("first preparation thread should not panic") + .expect("first preparation should succeed"); + let second = second + .join() + .expect("second preparation thread should not panic") + .expect("second preparation should succeed"); + + assert_eq!(first.action, WorkspaceAction::CloneTo(clone_dir.clone())); + assert_eq!(second.action, WorkspaceAction::UpdateAt(clone_dir)); + assert_eq!(repository.clone_calls.load(Ordering::SeqCst), 1); + assert_eq!(repository.update_calls.load(Ordering::SeqCst), 1); +} + +// Tests for find_workspace_root + +fn write_workspace_cargo_toml(dir: &Dir) { + dir.write("Cargo.toml", "[workspace]\nmembers = [\"crates/*\"]\n") + .expect("failed to write workspace Cargo.toml"); +} + +#[rstest] +fn find_workspace_root_finds_workspace_in_current_dir(temp_workspace: TempWorkspace) { + write_workspace_cargo_toml(&temp_workspace.dir); + assert_eq!( + find_workspace_root(&temp_workspace.path).expect("workspace root should be found"), + temp_workspace.path + ); +} + +#[rstest] +fn find_workspace_root_finds_workspace_in_parent_dir(temp_workspace: TempWorkspace) { + write_workspace_cargo_toml(&temp_workspace.dir); + let subdir = temp_workspace.path.join("crates").join("my_crate"); + temp_workspace + .dir + .create_dir_all("crates/my_crate") + .expect("failed to create workspace subdirectories"); + assert_eq!( + find_workspace_root(&subdir).expect("workspace root should be found from child crate"), + temp_workspace.path + ); +} + +#[rstest] +fn find_workspace_root_errors_when_no_workspace_found(temp_workspace: TempWorkspace) { + write_cargo_toml(&temp_workspace.dir, "not_a_workspace"); + let result = find_workspace_root(&temp_workspace.path); + assert!(matches!( + result.expect_err("non-workspace package should not have a workspace root"), + InstallerError::WorkspaceNotFound { .. } + )); +} + +#[path = "workspace_lock_workflow_tests.rs"] +mod workspace_lock_workflow_tests; diff --git a/installer/tests/behaviour_cli.rs b/installer/tests/behaviour_cli.rs index 0bd1f57c..b9ad4d13 100644 --- a/installer/tests/behaviour_cli.rs +++ b/installer/tests/behaviour_cli.rs @@ -16,10 +16,13 @@ use support::{ assert_cli_exits_successfully, assert_cli_exits_with_error, assert_dry_run_output_is_shown, assert_experimental_lint_dry_run_output_is_shown, assert_experimental_lint_opt_in_message_is_shown, assert_installation_succeeds_or_is_skipped, + assert_pinned_ref_output_is_shown, assert_ref_unsupported_message_is_shown, assert_suite_library_is_staged, assert_unknown_lint_message_is_shown, configure_dry_run_experimental_lint, configure_dry_run_experimental_lint_with_opt_in, - configure_dry_run_unknown_lint, configure_dry_run_with_target_dir, configure_suite_install, - is_toolchain_installed, pinned_toolchain_channel, run_installer_cli, workspace_root, + configure_dry_run_ref_in_workspace, configure_dry_run_unknown_lint, + configure_dry_run_with_pinned_ref, configure_dry_run_with_target_dir, + configure_ref_in_workspace, configure_suite_install, is_toolchain_installed, + pinned_toolchain_channel, run_installer_cli, workspace_root, }; #[given("the installer is invoked with dry-run and a target directory")] @@ -49,6 +52,21 @@ fn given_suite_install(cli_world: &CliWorld) { configure_suite_install(cli_world); } +#[given("the installer is invoked with dry-run and a pinned ref")] +fn given_dry_run_with_pinned_ref(cli_world: &CliWorld) { + configure_dry_run_with_pinned_ref(cli_world); +} + +#[given("the installer is invoked with a ref from a Whitaker workspace")] +fn given_ref_in_workspace(cli_world: &CliWorld) { + configure_ref_in_workspace(cli_world); +} + +#[given("the installer is invoked with dry-run and a ref from a Whitaker workspace")] +fn given_dry_run_ref_in_workspace(cli_world: &CliWorld) { + configure_dry_run_ref_in_workspace(cli_world); +} + #[when("the installer CLI is run")] fn when_installer_cli_run(cli_world: &CliWorld) { run_installer_cli(cli_world); @@ -94,6 +112,16 @@ fn then_suite_library_is_staged(cli_world: &CliWorld) { assert_suite_library_is_staged(cli_world); } +#[then("dry-run output shows the pinned ref")] +fn then_pinned_ref_output_is_shown(cli_world: &CliWorld) { + assert_pinned_ref_output_is_shown(cli_world); +} + +#[then("a ref-unsupported message is shown")] +fn then_ref_unsupported_message_is_shown(cli_world: &CliWorld) { + assert_ref_unsupported_message_is_shown(cli_world); +} + #[test] fn dry_run_reports_verbosity_levels() { let channel = pinned_toolchain_channel(); diff --git a/installer/tests/behaviour_cli/scenarios.rs b/installer/tests/behaviour_cli/scenarios.rs index c95291d8..c875b2e9 100644 --- a/installer/tests/behaviour_cli/scenarios.rs +++ b/installer/tests/behaviour_cli/scenarios.rs @@ -29,3 +29,18 @@ fn scenario_dry_run_rejects_experimental_lint_without_opt_in(cli_world: CliWorld fn scenario_dry_run_accepts_experimental_lint_with_opt_in(cli_world: CliWorld) { let _ = cli_world; } + +#[scenario(path = "tests/features/installer.feature", index = 23)] +fn scenario_pin_the_suite_to_a_ref_in_dry_run(cli_world: CliWorld) { + let _ = cli_world; +} + +#[scenario(path = "tests/features/installer.feature", index = 24)] +fn scenario_refuse_ref_inside_a_whitaker_workspace(cli_world: CliWorld) { + let _ = cli_world; +} + +#[scenario(path = "tests/features/installer.feature", index = 25)] +fn scenario_refuse_ref_during_dry_run_inside_a_whitaker_workspace(cli_world: CliWorld) { + let _ = cli_world; +} diff --git a/installer/tests/behaviour_cli/support.rs b/installer/tests/behaviour_cli/support.rs index 1bae4eac..f221e320 100644 --- a/installer/tests/behaviour_cli/support.rs +++ b/installer/tests/behaviour_cli/support.rs @@ -1,5 +1,14 @@ //! Shared fixtures, command helpers, and assertions for CLI behaviour tests. +#[path = "support/pinned_ref.rs"] +mod pinned_ref; + +pub(super) use pinned_ref::{ + assert_pinned_ref_output_is_shown, assert_ref_unsupported_message_is_shown, + configure_dry_run_ref_in_workspace, configure_dry_run_with_pinned_ref, + configure_ref_in_workspace, +}; + use super::prebuilt_markers::PREBUILT_INSTALL_MARKER; use rstest::fixture; use std::cell::{Cell, Ref, RefCell}; @@ -21,6 +30,8 @@ pub(super) struct CliWorld { toolchain: RefCell>, // Keep temp_dir alive for the lifetime of the scenario. temp_dir: RefCell>, + // Use an external working directory for scenarios that may pin a ref. + working_dir: RefCell>, } #[fixture] @@ -93,6 +104,12 @@ pub(super) fn setup_temp_dir(cli_world: &CliWorld) -> String { target_dir } +pub(super) fn use_external_working_dir(cli_world: &CliWorld) { + cli_world.working_dir.replace(Some( + TempDir::new().expect("failed to create working directory"), + )); +} + fn detect_host_target() -> Option { let output = Command::new("rustc").args(["-vV"]).output().ok()?; if !output.status.success() { @@ -201,7 +218,12 @@ pub(super) fn run_installer_cli(cli_world: &CliWorld) { let args = cli_world.args.borrow(); let mut command = Command::new(env!("CARGO_BIN_EXE_whitaker-installer")); command.args(args.iter()); - command.current_dir(workspace_root()); + let working_dir = cli_world.working_dir.borrow(); + command.current_dir( + working_dir + .as_ref() + .map_or_else(workspace_root, |dir| dir.path().to_owned()), + ); if cli_world.should_use_test_staged_suite.get() { command.env(TEST_STAGE_SUITE_ENV, "1"); } @@ -215,12 +237,16 @@ pub(super) fn get_output(cli_world: &CliWorld) -> Ref<'_, Output> { Ref::map(output, |opt| opt.as_ref().expect("output not set")) } +/// Borrows command output when a scenario has not been skipped. +fn output_for_assertions(cli_world: &CliWorld) -> Option> { + (!cli_world.skip_assertions.get()).then(|| get_output(cli_world)) +} + fn assert_exit_status(cli_world: &CliWorld, expected_success: bool) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); assert_eq!( output.status.success(), @@ -235,14 +261,13 @@ pub(super) fn assert_cli_exits_successfully(cli_world: &CliWorld) { } pub(super) fn assert_dry_run_output_is_shown(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; let toolchain = cli_world.toolchain.borrow(); let toolchain = toolchain.as_ref().expect("toolchain not set"); - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); assert!(stderr.contains("Dry run - no files will be modified")); @@ -267,11 +292,10 @@ pub(super) fn assert_cli_exits_with_error(cli_world: &CliWorld) { } pub(super) fn assert_unknown_lint_message_is_shown(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -289,11 +313,10 @@ pub(super) fn assert_unknown_lint_message_is_shown(cli_world: &CliWorld) { } pub(super) fn assert_experimental_lint_opt_in_message_is_shown(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); assert!( @@ -313,11 +336,10 @@ pub(super) fn assert_experimental_lint_opt_in_message_is_shown(cli_world: &CliWo } pub(super) fn assert_experimental_lint_dry_run_output_is_shown(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); assert!(stderr.contains("Dry run - no files will be modified")); @@ -332,11 +354,10 @@ pub(super) fn assert_experimental_lint_dry_run_output_is_shown(cli_world: &CliWo } pub(super) fn assert_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); assert!( output.status.success(), "installation failed: {}", @@ -345,11 +366,10 @@ pub(super) fn assert_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { } pub(super) fn assert_suite_library_is_staged(cli_world: &CliWorld) { - if cli_world.skip_assertions.get() { + let Some(output) = output_for_assertions(cli_world) else { return; - } + }; - let output = get_output(cli_world); let stderr = String::from_utf8_lossy(&output.stderr); let channel = cli_world.toolchain.borrow(); let channel = channel.as_ref().expect("toolchain not set"); diff --git a/installer/tests/behaviour_cli/support/pinned_ref.rs b/installer/tests/behaviour_cli/support/pinned_ref.rs new file mode 100644 index 00000000..95e66085 --- /dev/null +++ b/installer/tests/behaviour_cli/support/pinned_ref.rs @@ -0,0 +1,70 @@ +//! Pinned-ref setup and assertions for installer CLI behaviour scenarios. + +use super::{ + CliWorld, ensure_required_toolchain_available, output_for_assertions, setup_temp_dir, + use_external_working_dir, +}; + +/// The ref used by the pinned-install CLI scenarios. +const SCENARIO_REF: &str = "v0.2.5"; + +pub(crate) fn configure_dry_run_with_pinned_ref(cli_world: &CliWorld) { + let Some(channel) = ensure_required_toolchain_available(cli_world) else { + return; + }; + + let target_dir = setup_temp_dir(cli_world); + use_external_working_dir(cli_world); + cli_world.args.replace(vec![ + "--dry-run".to_owned(), + "--toolchain".to_owned(), + channel, + "--target-dir".to_owned(), + target_dir, + "--ref".to_owned(), + SCENARIO_REF.to_owned(), + ]); +} + +pub(crate) fn configure_dry_run_ref_in_workspace(cli_world: &CliWorld) { + cli_world.args.replace(vec![ + "--dry-run".to_owned(), + "--ref".to_owned(), + SCENARIO_REF.to_owned(), + ]); +} + +pub(crate) fn configure_ref_in_workspace(cli_world: &CliWorld) { + // The harness runs the installer from the Whitaker workspace root, so a + // `--ref` must be refused. `--skip-deps` keeps the refusal ahead of any + // dependency installation, and no toolchain is required to reach it. + cli_world.args.replace(vec![ + "--ref".to_owned(), + SCENARIO_REF.to_owned(), + "--skip-deps".to_owned(), + ]); +} + +pub(crate) fn assert_pinned_ref_output_is_shown(cli_world: &CliWorld) { + let Some(output) = output_for_assertions(cli_world) else { + return; + }; + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains(&format!("Pinned ref: {SCENARIO_REF}")), + "expected dry-run output to report the pinned ref, stderr: {stderr}" + ); +} + +pub(crate) fn assert_ref_unsupported_message_is_shown(cli_world: &CliWorld) { + let Some(output) = output_for_assertions(cli_world) else { + return; + }; + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("the current directory is itself a Whitaker workspace"), + "expected a ref-unsupported refusal, stderr: {stderr}" + ); +} diff --git a/installer/tests/behaviour_prebuilt.rs b/installer/tests/behaviour_prebuilt.rs index b51185f8..0bec3776 100644 --- a/installer/tests/behaviour_prebuilt.rs +++ b/installer/tests/behaviour_prebuilt.rs @@ -9,10 +9,14 @@ use std::sync::Mutex; use whitaker_installer::artefact::download::{ArtefactDownloader, DownloadError}; use whitaker_installer::artefact::extraction::{ArtefactExtractor, ExtractionError}; use whitaker_installer::cli::{Cli, InstallArgs}; +use whitaker_installer::git::CommitSha; use whitaker_installer::prebuilt::{PrebuiltConfig, PrebuiltResult, attempt_prebuilt_with}; use whitaker_installer::resolution::{CrateResolutionOptions, resolve_crates}; use whitaker_installer::test_utils::{prebuilt_manifest_json, sha256_hex}; +#[path = "behaviour_prebuilt/pinned_ref.rs"] +mod pinned_ref; + const FAKE_ARCHIVE: &[u8] = b"fake archive content"; const DEFAULT_TARGET: &str = "x86_64-unknown-linux-gnu"; const DEFAULT_TOOLCHAIN: &str = "nightly-2026-05-28"; @@ -115,6 +119,7 @@ struct PrebuiltWorld { should_attempt_prebuilt: Option, force_destination_conflict: bool, attempted_destination: Option, + expected_git_sha: Option, } #[fixture] @@ -224,11 +229,18 @@ fn when_prebuilt_attempted(world: &mut PrebuiltWorld) { .join("lib") }; world.attempted_destination = Some(destination_dir.clone()); + let expected_commit = world + .expected_git_sha + .as_deref() + .map(CommitSha::try_from) + .transpose() + .expect("full expected Git SHA"); let config = PrebuiltConfig { target, toolchain, destination_dir: &destination_dir, quiet: true, + expected_git_sha: expected_commit.as_ref(), }; let manifest_behaviour = world diff --git a/installer/tests/behaviour_prebuilt/pinned_ref.rs b/installer/tests/behaviour_prebuilt/pinned_ref.rs new file mode 100644 index 00000000..ab9c3ea1 --- /dev/null +++ b/installer/tests/behaviour_prebuilt/pinned_ref.rs @@ -0,0 +1,40 @@ +//! Pinned-ref BDD steps and scenarios for prebuilt installation. + +use super::{DEFAULT_TOOLCHAIN, FAKE_ARCHIVE, ManifestBehaviour, PrebuiltWorld, world}; +use rstest_bdd_macros::{given, scenario}; +use whitaker_installer::test_utils::{prebuilt_manifest_json, sha256_hex}; + +#[given("the pinned commit does not match the manifest git SHA")] +fn given_pinned_commit_mismatch(world: &mut PrebuiltWorld) { + world.expected_git_sha = Some("deadbeef00000000000000000000000000000000".to_owned()); +} + +#[given("the pinned commit matches the manifest git SHA")] +fn given_pinned_commit_matches(world: &mut PrebuiltWorld) { + let commit = "abc12340000000000000000000000000000000ab"; + world.expected_git_sha = Some(commit.to_owned()); + world.manifest_behaviour = Some(ManifestBehaviour::Ok( + prebuilt_manifest_json( + DEFAULT_TOOLCHAIN, + super::DEFAULT_TARGET, + sha256_hex(FAKE_ARCHIVE), + ) + .replacen("abc1234", commit, 1), + )); +} + +#[scenario( + path = "tests/features/prebuilt_download.feature", + name = "Prebuilt is skipped when the pinned ref does not match" +)] +fn scenario_pinned_ref_mismatch(world: PrebuiltWorld) { + let _ = world; +} + +#[scenario( + path = "tests/features/prebuilt_download.feature", + name = "Prebuilt succeeds when the pinned ref matches" +)] +fn scenario_pinned_ref_match(world: PrebuiltWorld) { + let _ = world; +} diff --git a/installer/tests/features/installer.feature b/installer/tests/features/installer.feature index b0b3626a..23ea9a85 100644 --- a/installer/tests/features/installer.feature +++ b/installer/tests/features/installer.feature @@ -134,3 +134,21 @@ Feature: Whitaker lint library installer When the installer CLI is run Then the CLI exits successfully And experimental lint dry-run output is shown + + Scenario: Pin the suite to a ref in dry-run + Given the installer is invoked with dry-run and a pinned ref + When the installer CLI is run + Then the CLI exits successfully + And dry-run output shows the pinned ref + + Scenario: Refuse --ref inside a Whitaker workspace + Given the installer is invoked with a ref from a Whitaker workspace + When the installer CLI is run + Then the CLI exits with an error + And a ref-unsupported message is shown + + Scenario: Refuse --ref during dry-run inside a Whitaker workspace + Given the installer is invoked with dry-run and a ref from a Whitaker workspace + When the installer CLI is run + Then the CLI exits with an error + And a ref-unsupported message is shown diff --git a/installer/tests/features/prebuilt_download.feature b/installer/tests/features/prebuilt_download.feature index dfa33f70..e4b5f37f 100644 --- a/installer/tests/features/prebuilt_download.feature +++ b/installer/tests/features/prebuilt_download.feature @@ -50,3 +50,19 @@ Feature: Prebuilt artefact download and verification Given the build-only flag is set When the install configuration is checked Then no prebuilt download is attempted + + Scenario: Prebuilt is skipped when the pinned ref does not match + Given a valid manifest for target "x86_64-unknown-linux-gnu" + And a matching archive with correct checksum + And the pinned commit does not match the manifest git SHA + When prebuilt download is attempted + Then the prebuilt result is fallback + And the fallback reason mentions "SHA mismatch" + + Scenario: Prebuilt succeeds when the pinned ref matches + Given a valid manifest for target "x86_64-unknown-linux-gnu" + And a matching archive with correct checksum + And the pinned commit matches the manifest git SHA + When prebuilt download is attempted + Then the prebuilt result is success + And the staging path uses toolchain, target, and lib directories