diff --git a/README.md b/README.md index aa1c983..7b6c5e1 100644 --- a/README.md +++ b/README.md @@ -3,30 +3,30 @@ A monolithic kernel work wrapper, to make it easy. > **Status: scaffold.** The command surface parses, every verb is wired -> through `main` to its module, and the paths resolve — but the bodies are -> `todo!()`. Twenty of them. Nothing builds a kernel, boots qemu, scrapes -> bugzilla, or sends mail yet. What exists is the shape of the thing and the -> seams to fill in, which is exactly what the *Where to pitch in* section -> below is for. +> through `main` to its module, and the paths resolve. Every body is a +> `todo!()` stub. Nothing builds a kernel, boots qemu, scrapes bugzilla, or +> sends mail yet. What exists is the shape of the tool and the seams to fill +> in, and *Where to pitch in* below is the list. ## Why The kernel patch lifecycle is a dozen commands you retype every time. Find something to fix, write it, run checkpatch, run it again after `--fix`, commit with the right trailer, build, boot it under qemu, diff against -master, work out who the maintainers are for those files, send. Then a week -later do it all again as v2, remembering to thread it off the original. +master, work out who maintains those files, send. Then a week later do it +all again as v2, remembering to thread it off the original. None of those steps are hard. They are just easy to get subtly wrong, and -the subtle mistakes — a missing `Signed-off-by`, a v2 that doesn't thread, -a missing CC — are the ones that get a patch ignored rather than reviewed. +the ones that get a patch ignored rather than reviewed are the boring +details: a missing `Signed-off-by`, a v2 that does not thread, a maintainer +who never got CC'd. -spectral is a thin wrapper over that loop. It is deliberately *thin*: every -verb shells out to a tool you already have and already trust. +spectral is a thin wrapper over that loop, and the thinness is the point. +Every verb shells out to a tool you already have and already trust. ## What it wraps -Nothing here reimplements kernel tooling, and nothing vendors it: +What each verb actually ends up running: | spectral | actually runs | |---|---| @@ -37,17 +37,19 @@ Nothing here reimplements kernel tooling, and nothing vendors it: | `kernel test` | `make`, then `qemu-system-x86_64` | | `kernel quest` | HTTP against `bugzilla.kernel.org` | -The two scripts are called out because it matters: they come from the tree -you are working in, so spectral cannot have a stale copy of the kernel's own -style rules or maintainer map. Upgrade your tree, get the new rules. +checkpatch.pl and get_maintainer.pl are worth calling out. They come from +the tree you are working in, so spectral cannot drift out of date with the +kernel's own style rules or maintainer map. Upgrade your tree and the new +rules apply. -And because it is thin, you can always drop the wrapper. `patch submit +Because it is thin, you can drop the wrapper at any point. `patch submit --dry-run` prints the recipients and the exact `git send-email` command line -instead of sending, so you can check the plumbing or just run it yourself. +instead of sending anything. Read it, then either drop `--dry-run` or run +the command yourself. ## The loop -What the finished tool should feel like — none of this runs yet: +What the finished tool should feel like. None of this runs yet: ```console $ spectral kernel quest @@ -81,7 +83,7 @@ prefix. ## Install -Needs a recent stable Rust — edition 2024, so **1.85 or newer**; developed +Needs a recent stable Rust: edition 2024, so 1.85 or newer. Developed against 1.98.1. ```console @@ -94,15 +96,15 @@ $ install -Dm755 target/release/spectral ~/.local/bin/spectral Not on crates.io, so `cargo install spectral` will get you something else. `cargo install --path .` from a clone works too. -Beyond Rust you want: +You also need: -- **a kernel tree** — for `scripts/checkpatch.pl` and - `scripts/get_maintainer.pl`. Both ship with the kernel; spectral does not - carry its own. -- **git with `send-email` configured** — `git send-email` must work from a - plain shell first. If it does not, `patch submit` cannot make it work, and - is not meant to. -- **qemu** — `qemu-system-x86_64` on `$PATH`, for `kernel test`. +- a kernel tree, for `scripts/checkpatch.pl` and + `scripts/get_maintainer.pl`. Both ship with the kernel, so spectral does + not carry its own. +- git with `send-email` configured. `git send-email` has to work from a plain + shell first. If it does not, spectral cannot fix it for you and is not + meant to try. +- qemu, so that `qemu-system-x86_64` is on `$PATH`, for `kernel test`. ## Configuration @@ -118,14 +120,14 @@ environment with `$HOME`-relative defaults: $ export SPECTRAL_KERNEL=$HOME/src/linux ``` -A tree is accepted only if it has `scripts/checkpatch.pl`; otherwise you get +A tree is accepted only if it has `scripts/checkpatch.pl`. Otherwise you get `not a kernel source tree` rather than a confusing failure three steps later. A missing tree reports the path and the variable that would have set it. The -tree is not required for `kernel quest`, which only needs the network. +tree is not needed for `kernel quest`, which only wants the network. `~/.config/spectral/config.toml` and a `spectral init` to clone the tree are -planned, not built — `src/config.rs` is the only file that will have to change -for either. +planned, not built. `src/config.rs` is the only file that has to change to +add either. ## Commands @@ -145,53 +147,62 @@ Commands: `spectral patch check` · `format` · `commit` · `create` · `submit` · `update` -Every one of them has `--help` that says more than this README does — and, -for now, a body that panics with a description of what it is supposed to do. +Every one of them has `--help` that says more than this README does, and for +now a body that panics with a description of what it is supposed to do. ## Where to pitch in -Every stub is a small, self-contained function with its signature and doc -comment already written, its caller already wired, and a `todo!()` naming the -command it should run. Pick one and the blast radius is that file. No stub -needs you to have read the rest of the crate. +Every stub is a small function that already has its signature, its doc +comment, and its caller in `main`. Pick one and you only need to read the +file it lives in. -Roughly in order of how much they unblock: +Roughly in order of how much they unblock. -**`patch submit` — the most self-contained win.** Needs a kernel tree but no -network and no scraper. +### patch submit -- `patch/maintainers.rs` · `lookup` — run `get_maintainer.pl --git` over the - patch, split the output into `To:` and `Cc:`, and collect the files the - patch touches -- `patch/maintainers.rs` · `add_cc` — fold `--cc` flags in without duplicates -- `patch/mod.rs` · `submit` — the above plus `git send-email`, with - `--dry-run` stopping one step short +The most self-contained win, and the only one that needs no network. It does +need a kernel tree and a working `git send-email`. -**`patch check` / `format` / `commit` / `create` — the everyday verbs.** -`patch/checkpatch.rs` · `run` already defines the `Target` enum (working -tree, a revision, or a patch file) and `Report` with an `is_clean`, so what -is missing is the process call and parsing the error/warning counts out of -its output. +- `patch/maintainers.rs` · `lookup` runs `get_maintainer.pl --git` over the + patch, splits the output into `To:` and `Cc:`, and collects the files the + patch touches. +- `patch/maintainers.rs` · `add_cc` folds `--cc` flags in without duplicates. +- `patch/mod.rs` · `submit` ties those two together with `git send-email`, + with `--dry-run` stopping one step short. -**`patch update` — small, and worth doing with a test.** `patch/mod.rs` · -`reroll_path` is the naming rule in one pure function: strip one leading -`vN-`, prepend `v-`. Re-running it at the same revision should be a -no-op, which is easier to assert than to describe. +### patch check, format, commit, create -**`kernel quest` — the fun one, if you like HTML.** `kernel/quest.rs` · -`Bugzilla::fetch_open` is the only place `reqwest` and `scraper` earn their -place in `Cargo.toml`; `run` then applies `--filter` and picks one. The -`QuestSource` trait is the seam for swapping bugzilla for syzbot or a lore -thread later. +The everyday verbs. `patch/checkpatch.rs` · `run` already has the `Target` +enum (working tree, a revision, or a patch file) and `Report` with +`is_clean`. What is missing is the process call and pulling the error and +warning counts out of checkpatch's output. -**`kernel test` — needs a machine you are willing to boot kernels on.** -`kernel/qemu.rs` · `build` then `boot`. Streaming serial output as it arrives -beats buffering it until qemu exits, which is what makes a boot hang -diagnosable. +### patch update -**`git.rs` — the four plumbing calls.** `current_branch`, `diff_against`, -`commit`, `rev_parse`, each a few lines over the `run` that is already -written. Take these if you want to warm up on something tiny. +Small, and worth writing a test for. `patch/mod.rs` · `reroll_path` holds the +naming rule in one pure function: strip one leading `vN-`, then prepend +`v-`. Running it twice at the same revision should do nothing, +which is easier to assert than to explain. + +### kernel quest + +The fun one if you like HTML. `kernel/quest.rs` · `Bugzilla::fetch_open` is +the only place `reqwest` and `scraper` earn their place in `Cargo.toml`, and +`run` then applies `--filter` and picks one issue. The `QuestSource` trait is +where you would add syzbot or a lore.kernel.org thread later. + +### kernel test + +Needs a machine you are willing to boot kernels on. `kernel/qemu.rs` · +`build` then `boot`. Streaming serial output as it arrives beats buffering it +until qemu exits, and it is the difference between a boot hang you can +diagnose and one you cannot. + +### git plumbing + +`git.rs` · `current_branch`, `diff_against`, `commit`, `rev_parse`. Each is a +few lines over the `run` that is already written, which makes these the +easiest place to start. There are no tests yet. The naming rule in `reroll_path` and the recipient split in `lookup` are the two that most want one. @@ -206,21 +217,21 @@ $ cargo test All three are clean on `main` and are the bar for a change. -A few conventions, in the spirit of keeping the crate reviewable: +A few conventions: -- `src/cli.rs` holds the whole command surface. It is one file on purpose — - the CLI is the specification, and reading it top to bottom should tell you - what the tool does. -- Command modules return what they produced; `main` prints it. Output policy, - formatting and all, lives in one place. -- Errors go through the one `Error` enum and `?`. The command paths have no - `unwrap`. -- A new stub gets a `todo!()` that names the command it will end up running. - If a whole module is unreachable until its caller exists, it carries a - single `#[allow(dead_code)]` with the reason attached — there are no - crate-wide allows, so the warnings come back as the stubs get filled in. -- No `unsafe`, no FFI. +- `src/cli.rs` holds the whole command surface. One file on purpose: the CLI + is the specification, and reading it top to bottom should tell you what the + tool does. +- Command modules return what they produced and `main` prints it, so output + formatting lives in one place. +- Errors go through the one `Error` enum and `?`. Nothing in the command + paths calls `unwrap`. +- A new stub gets a `todo!()` naming the command it will end up running. A + module that is unreachable until its caller exists carries a single + `#[allow(dead_code)]` with the reason attached. There are no crate-wide + allows, so the warnings come back as the stubs get filled in. +- No `unsafe` and no FFI. ## License -MIT — see `LICENSE`, © 2026 huntedbytheirs. +MIT. See `LICENSE`, © 2026 huntedbytheirs. diff --git a/src/cli.rs b/src/cli.rs index ff1ebe1..6bb9820 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,5 +1,5 @@ -//! The whole command surface, in one file, so the shape of the CLI reads at a -//! glance. Nothing here does any work — every variant is dispatched in `main`. +//! The whole command surface in one file, so the CLI is readable end to end. +//! Nothing here does any work: every variant is dispatched in `main`. use std::path::PathBuf; @@ -71,11 +71,11 @@ pub enum PatchCommand { Format(FormatArgs), /// Commit the work in progress with a kernel-style message Commit(CommitArgs), - /// Write the diff against the base branch out to .patch + /// Write the diff against the base branch out to a .patch file Create(CreateArgs), /// Send a patch, with To/CC taken from get_maintainer.pl Submit(SubmitArgs), - /// Re-roll a patch as v, renaming the file to match + /// Re-roll a patch as vN, renaming the file to match Update(UpdateArgs), } diff --git a/src/config.rs b/src/config.rs index 0776db7..f43a0ee 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,7 +1,7 @@ //! Where the kernel tree and the patches live. //! -//! Everything else asks this module for paths, so the day a config file or -//! `spectral init` arrives, it is the only thing that changes. +//! Everything else asks this module for paths, so adding a config file or a +//! `spectral init` only touches this file. #![allow(dead_code)] // accessors are read once the patch verbs stop being stubs @@ -29,9 +29,9 @@ impl Config { /// Resolve the tree from `$SPECTRAL_KERNEL`, falling back to /// `~/.spectral/linux`. /// - /// The tree is not checked for existence here — commands that need it call - /// [`Config::require_kernel_tree`], so `kernel quest` still works before - /// anything is cloned. + /// The tree is not checked for existence here. Commands that need it call + /// [`Config::require_kernel_tree`], which leaves `kernel quest` working + /// before anything is cloned. /// /// TODO: also read `~/.config/spectral/config.toml` once `spectral init` /// exists. The environment variable should keep winning over the file. @@ -56,7 +56,7 @@ impl Config { &self.patch_dir } - /// The kernel tree, having confirmed it is one. + /// Resolve the tree and check that it really is a kernel source tree. pub fn require_kernel_tree(&self) -> Result<&Path> { if self.kernel_tree.join("scripts/checkpatch.pl").is_file() { Ok(&self.kernel_tree) diff --git a/src/error.rs b/src/error.rs index 9dcfd89..d950211 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,7 @@ //! One error type for the whole CLI. //! -//! Anything that can go wrong ends up as an [`Error`], gets a `?` at the call -//! site, and is printed once in `main` — no `unwrap` in the command paths. +//! Everything that can go wrong ends up as an [`Error`], gets a `?` at the +//! call site, and is printed once by `main`. use std::path::PathBuf; diff --git a/src/git.rs b/src/git.rs index 8745e2d..66aff86 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,6 +1,6 @@ //! Thin plumbing over the `git` binary. //! -//! Nothing in here knows what a kernel is: it starts processes, hands back +//! Nothing in here knows what a kernel is. It starts processes, returns //! stdout, and turns a non-zero exit into an [`Error`]. #![allow(dead_code)] // reachable as soon as the patch verbs stop being stubs @@ -28,8 +28,8 @@ impl Git { /// Run git in the repository and return its trimmed stdout. /// - /// This is the only place a git process is started; the semantic - /// operations below are written in terms of it. + /// This is the only place a git process is started. Every other method + /// here goes through it. pub fn run(&self, args: &[&str]) -> Result { let output = Command::new("git") .arg("-C") diff --git a/src/kernel/qemu.rs b/src/kernel/qemu.rs index 81c0b7e..a3a45c6 100644 --- a/src/kernel/qemu.rs +++ b/src/kernel/qemu.rs @@ -1,4 +1,4 @@ -//! `spectral kernel test` — build the tree, then boot it under qemu. +//! `spectral kernel test`: build the tree, then boot it under qemu. #![allow(dead_code)] // nothing is reachable until `run` stops being a stub diff --git a/src/kernel/quest.rs b/src/kernel/quest.rs index 9c87e58..7f7e8b5 100644 --- a/src/kernel/quest.rs +++ b/src/kernel/quest.rs @@ -1,8 +1,7 @@ -//! `spectral kernel quest` — go and find something worth fixing. +//! `spectral kernel quest`: go and find something worth fixing. //! -//! The source is behind [`QuestSource`] so swapping bugzilla for syzbot, a -//! lore.kernel.org thread, or a local TODO file is one new impl and no change -//! to the command. +//! The source sits behind [`QuestSource`], so adding a local TODO file or a +//! syzbot scraper later means one new impl and no change to the command. #![allow(dead_code)] // nothing is reachable until `run` stops being a stub diff --git a/src/main.rs b/src/main.rs index e136eef..f3fec19 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,8 @@ -//! spectral — a monolithic kernel work wrapper to make it easy. +//! spectral: a monolithic kernel work wrapper to make it easy. //! -//! `main` does three things: parse, dispatch, render. Anything with a kernel -//! or git in it lives behind one of the modules below. +//! `main` parses the command line, dispatches to a module, and prints what +//! comes back. Anything that touches a kernel or a git repository lives in one +//! of the modules below. mod cli; mod config; @@ -31,8 +32,8 @@ async fn main() -> ExitCode { } } -/// The command modules do the work and hand back what they produced; printing -/// it is `main`'s job. +/// Command modules do the work and return what they produced. Printing it is +/// `main`'s job. async fn run(cli: Cli) -> Result<()> { let config = Config::load()?; diff --git a/src/patch/checkpatch.rs b/src/patch/checkpatch.rs index 658be14..08ddb88 100644 --- a/src/patch/checkpatch.rs +++ b/src/patch/checkpatch.rs @@ -1,4 +1,4 @@ -//! `scripts/checkpatch.pl`, run the two ways spectral needs it. +//! `scripts/checkpatch.pl`, from the kernel tree you are working in. #![allow(dead_code)] // reachable as soon as the patch verbs stop being stubs @@ -36,9 +36,9 @@ impl Report { /// Run checkpatch.pl over `target`. /// -/// `fix` adds `--fix`, which rewrites the patch file in place — only -/// `Target::File` supports it, and the caller is responsible for having the -/// change committed first so a bad fix is one `git checkout` away. +/// `fix` adds `--fix`, which rewrites the patch file in place. Only +/// `Target::File` supports it, and the caller has to have the change +/// committed first so a bad fix can be reverted. pub fn run(kernel_tree: &Path, target: &Target, strict: bool, fix: bool) -> Result { todo!( "scripts/checkpatch.pl --no-tree in {} on {target:?} (strict={strict}, fix={fix})", diff --git a/src/patch/maintainers.rs b/src/patch/maintainers.rs index d886645..9cd3cb8 100644 --- a/src/patch/maintainers.rs +++ b/src/patch/maintainers.rs @@ -19,8 +19,8 @@ pub struct Recipients { /// Look up recipients for a patch. /// -/// Runs `get_maintainer.pl --roles=... --git` over the patch's diff, which is -/// what gives us the files it touches as a side effect. +/// Runs `get_maintainer.pl --roles=... --git` over the patch's diff. That also +/// gives us the files the patch touches. pub fn lookup(kernel_tree: &Path, patch: &Path) -> Result { todo!("get_maintainer.pl --git on {patch:?} inside {kernel_tree:?}") } diff --git a/src/patch/mod.rs b/src/patch/mod.rs index 1ef7130..3d7f38c 100644 --- a/src/patch/mod.rs +++ b/src/patch/mod.rs @@ -1,7 +1,6 @@ -//! `spectral patch …` — carry a change from working tree to mailing list. +//! `spectral patch`: carry a change from working tree to mailing list. //! -//! The verbs are all stubs, but the order they are meant to be run in is the -//! point of the module: +//! The verbs are all stubs. The order they are meant to be run in is: //! //! ```text //! check ─▶ format ─▶ commit ─▶ create ─▶ submit @@ -9,8 +8,8 @@ //! └─ update┘ (v2, v3, …) //! ``` //! -//! Each verb resolves the tree through [`Config::require_kernel_tree`] and -//! builds a [`crate::git::Git`] over it; that is how they reach the plumbing. +//! Each verb gets the tree from [`Config::require_kernel_tree`] and builds a +//! [`crate::git::Git`] over it, which is how it reaches the plumbing. pub mod checkpatch; pub mod maintainers; @@ -51,9 +50,8 @@ pub fn create(config: &Config, args: CreateArgs) -> Result { /// Send a patch to whoever `get_maintainer.pl` names. /// -/// `--dry-run` stops one step short: print the recipients and the exact -/// `git send-email` invocation instead of sending it, which is the honest way -/// to review a first submission to a list. +/// With `--dry-run` it prints the recipients and the exact `git send-email` +/// invocation instead of sending anything. pub fn submit(config: &Config, args: SubmitArgs) -> Result<()> { let _ = (config, args); todo!("look up recipients, then git send-email")