Repository navigation
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
whitaker binary behind an internal library boundary (3.5.1)whitaker binary behind an internal library boundary (3.5.1)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
9b758d1 to
afb1e4a
Compare
Draft the execution plan for roadmap item 3.5.1: add a real `whitaker` binary at the root package and move the current installer orchestration behind an internal library boundary. The plan places the CLI domain, ports, and adapters in a new `crates/whitaker_cli` crate rather than in the root package, because the root package's library requires `feature(rustc_private)` and is excluded from `make test` by `TEST_EXCLUDES`. The root `src/main.rs` becomes a thin composition root. Scope is limited to `install` and `ls`; `check` and `doctor`, the configuration model, and the deprecation shim remain with their own roadmap items. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A six-perspective review invalidated the first draft's structural bet. `cargo package -p whitaker` fails: the root package depends on four `publish = false` rustc shim crates and lacks crates.io metadata, so `cargo install whitaker` was unreachable. Extracting the Dylint driver library into `crates/whitaker_lint_core` is therefore a precondition, not a contingency, and it is now milestone EP-M0. That removes the need for a separate CLI crate, makes `CARGO_BIN_EXE_whitaker` available to the end-to-end tests, and confines `ortho_config` to the CLI binary. Record the `whitaker` name collision the first draft missed: the installer generates an executable named `whitaker` that a `cargo install`ed binary deterministically shadows, breaking `whitaker --all` and this repository's own `make lint`. The binary now forwards unrecognized arguments to `cargo dylint`. Cut both original verification obligations as vacuous. Nothing inverts a release-asset name, and clap rejects the flag conflicts before any repository-owned routing code runs. Verify argv classification totality and disjointness instead, bounded by Kani and closed unbounded by Verus. Settle exit codes at 0, 2, and 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move compiler-private driver code and its tests into the non-publishable `whitaker_lint_core` package so the root package can become a normal, publishable CLI package. Migrate all in-tree lint consumers and templates to the internal package, and retain the moved tests through a dedicated feature-free core pass. This avoids Cargo feature-unification linkage failures without weakening the all-feature Dylint validation.
Move command routing and installation flow behind `whitaker_installer::orchestration` while keeping the legacy binary as a small composition root. Add a normalized `--dry-run` output snapshot to preserve the existing installer contract through the relocation.
Document the five unchanged commits replayed onto harden-lint-config. Record lockfile regeneration and passing format, typecheck, lint, Markdown and Mermaid checks. Retain the failed UI example-capture evidence and upstream Dylint issue. Keep compiler admission intact and mark the full local test suite as blocked while the root CLI implementation remains at M1.
cdcf4ee to
04f6fc4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Capture the no-op rebase readback and distinguish passing lint, formatting, and typecheck gates from the still-failing repository test gate. Keep roadmap item 3.5.1 open while M2 through M4 remain.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The target-to-head diff exceeds the ExecPlan 70-file tolerance before M2 through M4. Record the evidence, pause implementation pending approval, and keep roadmap item 3.5.1 unchecked until its requirements land.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Update the living plan with the user's decision to resume the approved milestones despite the pre-existing file-count tolerance breach. Preserve the remaining limits and record the current validation failure and verified CLI parsing API.
|
The generated The wrapper may also need the same guard semantics through an approved owner follow-up; that work need not wait for M2 to merge if it can be reviewed independently. Once the Dylint interface is published, record its actual supported invocation here or in that narrow follow-up. Preserve the current argv and exit-status behavior. This is coordination only. Installer Evidence: Whitaker wrapper at immutable main revision Asset production, relocation qualification and provisioning are tracked separately in #496. That issue requires platform executable suffixes ( |
Add install and inventory commands, preserve cargo-dylint forwarding, and expose the installer through a library boundary. Include binstall metadata, verification harnesses, behavioural coverage, and the architecture and publishing documentation.
Record successful staging paths so the root command can forward Dylint invocations from outside the repository while preserving explicit user overrides. Keep legacy installer record failures non-fatal and root CLI failures fatal where forwarding requires the record. Add regression coverage for override, invalid path, and external-directory behaviour. Complete the implementation documentation and record the remaining Verus and crates.io reservation criteria.
Recover malformed installation records only after a successful install, normalize the dry-run snapshot using the pinned toolchain, and gate the XDG-specific forwarding test to supported targets. Document the selected MIT license option for the vendored test helper and record final implementation status in the ExecPlan.
Clarify corrupt installation-record recovery, document root CLI precedence, and reset coverage instrumentation before composing both test passes. Record final review and validation status in the ExecPlan.
There was a problem hiding this comment.
Gates Failed
(3 files with Code Duplication, String Heavy Function Arguments, Complex Method)
Our agent can fix these. Install it.
Gates Passed
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| binstall_metadata.rs | 2 advisory rules | 10.00 → 9.10 | Suppress |
| property_arg_parity.rs | 1 advisory rule | 9.39 | Suppress |
| behaviour_binstall.rs | 1 advisory rule | 9.69 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
| /// assert!(url.contains("/whitaker-x86_64-unknown-linux-gnu-v0.2.7.tgz")); | ||
| /// ``` | ||
| #[must_use] | ||
| pub fn expand_pkg_url_for(package: &str, version: &str, target: &str) -> String { |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 2 functions with similar structure: expand_bin_dir_for,expand_pkg_url_for
| /// ``` | ||
| #[must_use] | ||
| pub fn expand_pkg_url(version: &str, target: &str) -> String { | ||
| expand_pkg_url_for("whitaker-installer", version, target) |
There was a problem hiding this comment.
❌ New issue: String Heavy Function Arguments
In this module, 83.3% of all arguments to its 7 functions are strings. The threshold for string arguments is 39.0%
| fn validate_whitaker_package_metadata() -> Result<(), String> { | ||
| let manifest_path = Path::new(env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"); | ||
| let manifest = load_cargo_toml_from(&manifest_path)?; | ||
| let package = required_table(&manifest, "package")?; | ||
| let package_name = required_string(package, "name")?; | ||
| let version = required_string(package, "version")?; | ||
| let binstall = extract_binstall_table(&manifest)?; | ||
|
|
||
| if package_name != "whitaker" { | ||
| return Err(format!("root package name changed to `{package_name}`")); | ||
| } | ||
| if required_string(&binstall, "pkg-url")? != PKG_URL_TEMPLATE { | ||
| return Err("root pkg-url must match the shared template".to_owned()); | ||
| } | ||
| if required_string(&binstall, "bin-dir")? != BIN_DIR_TEMPLATE { | ||
| return Err("root bin-dir must match the shared template".to_owned()); | ||
| } | ||
| if required_string(&binstall, "pkg-fmt")? != DEFAULT_PKG_FMT { | ||
| return Err("root package default archive format changed".to_owned()); | ||
| } | ||
|
|
||
| let overrides = required_table(&binstall, "overrides")?; | ||
| let windows = required_table(overrides, WINDOWS_OVERRIDE_TARGET)?; | ||
| if required_string(windows, "pkg-fmt")? != WINDOWS_PKG_FMT { | ||
| return Err("root Windows archive override changed".to_owned()); | ||
| } | ||
| if overrides.len() != 1 { | ||
| return Err(format!( | ||
| "expected one root override, found {}", | ||
| overrides.len() | ||
| )); | ||
| } | ||
|
|
||
| for target in RELEASE_TARGETS { | ||
| let whitaker_url = expand_pkg_url_for(package_name, version, target); | ||
| let installer_url = expand_pkg_url_for("whitaker-installer", version, target); | ||
| if whitaker_url == installer_url { | ||
| return Err(format!( | ||
| "package URLs must remain distinct for release target {target}" | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
❌ New issue: Complex Method
validate_whitaker_package_metadata has a cyclomatic complexity of 9, threshold = 9
| const fn bit(self) -> u16 { | ||
| match self { | ||
| Self::TargetDir => 1 << 0, | ||
| Self::Lint => 1 << 1, | ||
| Self::IndividualLints => 1 << 2, | ||
| Self::Experimental => 1 << 3, | ||
| Self::Jobs => 1 << 4, | ||
| Self::Toolchain => 1 << 5, | ||
| Self::Cranelift => 1 << 6, | ||
| Self::DryRun => 1 << 7, | ||
| Self::Verbosity => 1 << 8, | ||
| Self::Quiet => 1 << 9, | ||
| Self::SkipDeps => 1 << 10, | ||
| Self::SkipWrapper => 1 << 11, | ||
| Self::NoUpdate => 1 << 12, | ||
| Self::BuildOnly => 1 << 13, | ||
| } | ||
| } |
There was a problem hiding this comment.
❌ New issue: Code Duplication
The module contains 2 functions with similar structure: OptionKind.bit,OptionKind.name
This branch has not been deployed
1 participant
Summary
This branch delivers roadmap task 3.5.1 by adding the root
whitakercommand and moving installer orchestration behind a library boundary. It keeps compiler-private Dylint code in an internal crate, preserveswhitaker-installer, supportsinstalland transitionalls --json, and forwards legacy lint arguments tocargo dylint.The root command records successful staging paths by toolchain so legacy invocations work outside the installation checkout; an explicit
DYLINT_LIBRARY_PATHstill takes precedence. The installer avoids generating a competingwhitakerwrapper.Roadmap task: 3.5.1
ExecPlan: docs/execplans/3-5-1-root-whitaker-binary.md
Review walkthrough
Validation
make check-fmt: passed.make test: passed; 1,699 workspace tests, five skipped, the separate 80-test feature-free core suite, and doctests passed.make typecheckandmake lint: passed.make markdownlint,make nixie, andmbake validate Makefile: passed.cargo package -p whitaker --list --allow-dirty: passed.Notes
make coverageattempt timed out on a trybuild case after three retries and produced no LCOV report. The coverage target now clears workspace instrumentation data before its passes; that revised optional target has not been rerun end-to-end.References