xtask/src/fmt.rs
//! `cargo xtask fmt` — the only correct way to format this repository.
//!
//! # `cargo fmt --all` is a landmine, in both directions
//!
//! Its own `--help` says what it does: *"Format all packages, and also their
//! **local path-based dependencies**"*. It follows those transitively, and it
//! honours nothing that would stop it — not `optional`, not features, not the
//! workspace `exclude` list, not `Cargo.lock`. `cargo-fmt` re-runs
//! `cargo metadata --no-deps` at each hop and walks every `dependencies[].path`
//! it finds.
//!
//! From this repository that walk used to escape on its very first hop:
//!
//! ```text
//! gunnar-cli (a member)
//! └── the old znippy engine crate optional dep, AND in the workspace `exclude`
//! ├── ../../../znippy/znippy-common
//! │ └── ../../znippy-zoomies/… → and onward
//! ├── ../../../znippy/znippy-compress
//! ├── ../../../znippy/plugins/native/znippy-plugin-git
//! └── ../../../znippy-zoomies
//! ```
//!
//! The `exclude` entry on the old znippy engine crate is the mechanism the root manifest
//! relies on to keep znippy out of the resolved graph — and `cargo fmt` ignores
//! it completely. `cargo metadata --no-deps` reports 17 packages, every one of
//! them inside this tree, and there is no parent workspace, so **nothing about
//! the workspace definition warns you.**
//!
//! ⚠ **That first hop was removed on 2026-08-05** — `gunnar-cli` no longer
//! names the old znippy engine crate at all (see its manifest, and
//! `crates/gunnar-znippy/Cargo.toml` for where the znippy engine went). No workspace
//! member declares a path dependency leaving this repository any more, which is
//! read off the manifests rather than off a rustfmt run: running `--all` to
//! confirm is the command this file exists to refuse. The measurement below is
//! therefore kept as the RECORD of what that edge cost while it existed, not as
//! a claim about today's tree. The narrowness argument underneath it is
//! unaffected and is why this task still exists: `--all` never reached the
//! detached trees, and there are four of them now.
//!
//! MEASURED here on 2026-08-04, rustfmt 1.9.0-stable, with the edge in place:
//!
//! | `cargo fmt --all --check` | diffs |
//! |---|---|
//! | facett | 8459 |
//! | znippy-zoomies | 2614 |
//! | znippy | 1262 |
//! | Skidbladnir | 218 |
//! | nornir | 3 |
//! | **five repositories gunnar does not own** | **12556** |
//! | this repository | 583 |
//!
//! The writing form is not hypothetical: on 2026-08-04 it rewrote 680 files in
//! facett, 76 in znippy and 167 in znippy-zoomies — applying this repo's
//! edition-2021 import sorting to edition-2024 repositories — and all of it had
//! to be reverted from saved patches.
//!
//! And it is too NARROW at the same time. Per package manifest this repository
//! had **1191** diffs, not 583. The missing 608 were `bench/` (500),
//! the old znippy engine bench (61) and `installtest/` (47): trees that no
//! traversal from the root ever reaches, because none of them is in `members`
//! and they are detached workspaces of their own. `crates/gunnar-znippy` joined
//! them on 2026-08-05 and is unreachable for the same reason.
//!
//! # So: discovery is a filesystem walk, and formatting is per manifest
//!
//! [`package_manifests`] finds every manifest with a `[package]` table by
//! walking the repository, which is the only method that sees all three of the
//! detached trees. `--manifest-path` was MEASURED not to leak: formatting all
//! 21 manifests produced zero diffs outside this worktree.
//!
//! # Why this shells out to `cargo-fmt` and not to `cargo fmt`
//!
//! `.cargo/config.toml` aliases `fmt` to this task, so `cargo fmt` from inside
//! the repository lands here instead of on the landmine. That makes
//! `cargo fmt …` a recursive call *from* here — so this module invokes the
//! `cargo-fmt` binary directly, which is the same program without the alias in
//! front of it. [`cargo_fmt_binary`] finds it next to `$CARGO` first, PATH
//! second.
use std::path::{Path, PathBuf};
use std::process::Command;
/// Check, or rewrite in place.
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum Mode {
/// `--check`: report and fail, change nothing.
Check,
/// Rewrite the files.
Write,
}
/// Directory names never descended into.
///
/// * `target` / `.git` — build and VCS state, not source.
/// * `xtask/tests/fixtures` — deliberately-BROKEN workspaces, each one the
/// red-first proof of a dependency-guard rule (`gunnar-store` depending on
/// `gunnar-client`, and so on). They are fixtures asserted against, not code
/// whose style this repository owns, and they are in the root `exclude` list
/// for the same reason.
/// * `.idea` and `.claude` — tooling state, and NOT this repository's source.
/// Each one's `worktrees/` holds FULL checkouts of sibling repositories
/// (edda, draupnir, znippy, …) parked there by local tooling. Descending
/// into them puts other repositories' trees into this repository's fmt set
/// — the exact cross-repo rewrite this module exists to prevent (measured
/// 2026-08-04: 12556 diffs in five repos), reachable this time without even
/// leaving the checkout.
///
/// `.idea` was found 2026-08-16 when `tests/fmt.rs` flagged edda guard
/// fixtures arriving through it. `.claude` is the same thing one directory
/// over: the tooling moved, the entry did not follow, and on 2026-09-12 the
/// same test failed again naming
/// `.claude/worktrees/edda/crates/korp-installer/tests/fixtures/…`. Both are
/// listed rather than a pattern like "any dotted directory", because the
/// rule being stated is *these are other repositories' checkouts*, and a
/// dotted directory that holds our own source one day should fail loudly
/// rather than be skipped silently.
const SKIP_DIRS: &[&str] = &["target", ".git", ".idea", ".claude"];
/// Trees this repository holds but does not own the STYLE of, matched on a path
/// suffix so the check is OS-agnostic and independent of where the repository is
/// checked out.
///
/// * `xtask/tests/fixtures` — the deliberately-broken guard fixtures described
/// above.
/// * `vendor/` — third-party source held verbatim. `vendor/gix-features` is
/// gix-features 0.49.0 *verbatim plus six lines*, carried as a `[patch]`
/// until the fix is released upstream, and "verbatim plus six lines" is the
/// entire point of it: rustfmt would rewrite the other several thousand and
/// nobody could diff it against upstream again. It is also edition 2024,
/// where this repo is 2021, so it would get a DIFFERENT import sort — which
/// is precisely the cross-repository damage this module exists to prevent,
/// reproduced inside our own tree.
const SKIP_PATHS: &[&str] = &["xtask/tests/fixtures", "/vendor/"];
fn is_skipped(path: &Path) -> bool {
let p = path.to_string_lossy().replace('\\', "/");
SKIP_PATHS.iter().any(|s| p.contains(s))
}
/// Every manifest under `root` that declares a `[package]`, sorted.
///
/// A **filesystem walk**, deliberately, rather than the root `members` list or
/// `cargo metadata`. Both of those stop at the workspace boundary and would
/// miss the old znippy engine crate (workspace `exclude`), `bench/` and
/// the old znippy engine bench (detached workspaces) and `installtest/` (not
/// a member) — 608 diffs' worth of files that had never been formatted at all.
///
/// The `[package]` filter is also what drops the virtual root manifest, which
/// `cargo fmt --manifest-path` rejects with a usage error rather than treating
/// as a no-op.
pub fn package_manifests(root: &Path) -> Vec<PathBuf> {
let mut out = Vec::new();
collect(root, &mut out);
out.sort();
out
}
fn collect(dir: &Path, out: &mut Vec<PathBuf>) {
let Ok(rd) = std::fs::read_dir(dir) else {
return;
};
for entry in rd.flatten() {
let path = entry.path();
if path.is_dir() {
let name = entry.file_name();
let name = name.to_string_lossy();
if SKIP_DIRS.iter().any(|s| *s == name) || is_skipped(&path) {
continue;
}
collect(&path, out);
} else if path.file_name().and_then(|n| n.to_str()) == Some("Cargo.toml")
&& !is_skipped(&path)
&& declares_a_package(&path)
{
out.push(path);
}
}
}
/// Does this manifest have a `[package]` table?
///
/// A virtual manifest (the repository root, and the root of a detached
/// workspace that is only a `[workspace]`) has none, and `cargo fmt
/// --manifest-path` on one exits with a usage error.
fn declares_a_package(manifest: &Path) -> bool {
let Ok(text) = std::fs::read_to_string(manifest) else {
return false;
};
text.lines().any(|l| l.trim() == "[package]")
}
/// The real `cargo-fmt`, bypassing the `fmt` alias that points at this task.
///
/// Next to `$CARGO` first — that is the toolchain actually in use, and on a
/// rustup install both binaries sit in the same directory — then a bare
/// `cargo-fmt` for PATH resolution.
pub fn cargo_fmt_binary() -> PathBuf {
if let Ok(cargo) = std::env::var("CARGO") {
let sibling = Path::new(&cargo).with_file_name("cargo-fmt");
if sibling.is_file() {
return sibling;
}
}
PathBuf::from("cargo-fmt")
}
/// Why `cargo-fmt` exited non-zero for one manifest.
///
/// **The distinction this enum exists for.** `cargo-fmt` returns non-zero for
/// two completely unrelated reasons, and `run` used to push both onto one list
/// called `bad` and print all of them under "NOT FORMATTED":
///
/// * rustfmt found diffs — the code really is not formatted;
/// * `cargo metadata` could not load the manifest at all, because it names a
/// sibling repository by relative path that is not beside THIS checkout.
///
/// The second is not a formatting verdict. Not one byte of the tree was
/// examined, and the advice that followed it — "Fix with `cargo xtask fmt
/// --write`" — writes nothing, because the writing run fails at exactly the
/// same place. That is an absence rendered as a value, in the step every
/// quoted suite number is gated behind: `xtask ci` reported `FAILED at fmt` on
/// a tree whose every file was rustfmt-clean.
///
/// Measured from a lane worktree at
/// `~/scratch/cargo/<lane>/gunnar` on 2026-08-05: 4 of 23
/// manifests reported "NOT FORMATTED", all four unreadable rather than
/// unformatted (`bench`, the old znippy engine crate,
/// the old znippy engine bench, `installtest`), naming `draupnir` and
/// `znippy-common`.
#[derive(Debug, PartialEq, Eq)]
enum Verdict {
/// rustfmt looked and found diffs.
NotFormatted,
/// `cargo metadata` could not load the manifest. Nothing was examined.
Unreadable(String),
}
/// Which of the two `cargo-fmt` just reported.
///
/// Decided on `cargo metadata`'s own sentence, which `cargo-fmt` passes
/// through verbatim, because there is nothing else to decide on: both failures
/// are a non-zero exit and rustfmt's own diff output goes to stdout while this
/// goes to stderr. A message that stops matching lands on `NotFormatted`, which
/// is the behaviour before this existed — a false red, never a false green.
fn classify(stderr: &str) -> Verdict {
if stderr.contains("`cargo metadata` exited with an error")
|| stderr.contains("failed to load manifest for dependency")
{
// The most specific line wins. `failed to read <path>` names the file
// that is not there, which is what an operator acts on; the
// `failed to load manifest for dependency` line above it only names
// the crate. Falling back the other way round would print the vaguer
// of the two whenever both are present, which is always.
let cause = stderr
.lines()
.find(|line| line.contains("failed to read"))
.or_else(|| {
stderr
.lines()
.find(|line| line.contains("failed to load manifest"))
})
.unwrap_or("cargo metadata failed")
.trim()
.to_owned();
Verdict::Unreadable(cause)
} else {
Verdict::NotFormatted
}
}
/// Format (or check) every package in the repository. `true` if all passed.
///
/// Reports per manifest rather than as one verdict: "the repository is not
/// formatted" sends you looking, "`bench/Cargo.toml` is not formatted" does not.
///
/// A manifest cargo cannot even load is reported **separately** and never as a
/// formatting verdict — see [`Verdict`]. Whether that fails the run follows the
/// same rule `ci`'s znippy-engine steps already use, through the same
/// [`worktree::main_worktree`](crate::worktree::main_worktree): a machine that
/// simply has no sibling checkout cannot check those trees and says so, while a
/// LINKED WORKTREE whose main checkout does have them is refused, because a
/// pass there would cover less than the same command in the main checkout and
/// nothing in the output would say so.
pub fn run(root: &Path, mode: Mode) -> bool {
let manifests = package_manifests(root);
if manifests.is_empty() {
eprintln!(
"xtask fmt: found no package manifest under {}",
root.display()
);
eprintln!(" That cannot be right, and a silent zero-work success is the one");
eprintln!(" outcome this must never report. Failing instead.");
return false;
}
let program = cargo_fmt_binary();
let mut unformatted = Vec::new();
let mut unreadable: Vec<(String, String)> = Vec::new();
for manifest in &manifests {
let rel = manifest.strip_prefix(root).unwrap_or(manifest);
let mut cmd = Command::new(&program);
cmd.current_dir(root).arg("--manifest-path").arg(manifest);
if mode == Mode::Check {
cmd.arg("--check");
}
// `output`, not `status`: the stderr IS the evidence for which of the
// two failures this is, and with `status` it went straight to the
// terminal where nothing could read it.
match cmd.output() {
Ok(out) if out.status.success() => {}
Ok(out) => {
let stderr = String::from_utf8_lossy(&out.stderr);
match classify(&stderr) {
Verdict::NotFormatted => {
// rustfmt's diff is the useful part of a real failure
// and used to reach the terminal directly; keep it.
print!("{}", String::from_utf8_lossy(&out.stdout));
eprint!("{stderr}");
unformatted.push(rel.display().to_string());
}
Verdict::Unreadable(cause) => {
unreadable.push((rel.display().to_string(), cause))
}
}
}
Err(e) => {
eprintln!("xtask fmt: could not run {}: {e}", program.display());
eprintln!(" Install it with `rustup component add rustfmt`.");
return false;
}
}
}
// ── the manifests nothing could look at ────────────────────────────────
let unreadable_ok = if unreadable.is_empty() {
true
} else {
let main = crate::worktree::main_worktree(root);
eprintln!(
"\nxtask fmt: NOT CHECKED, {} of {} package(s) — cargo could not load the manifest,",
unreadable.len(),
manifests.len()
);
eprintln!(" so rustfmt never looked at them. This is NOT a formatting verdict:");
for (name, cause) in &unreadable {
eprintln!(" {name}");
eprintln!(" {cause}");
}
match &main {
Some(main) => {
eprintln!(
"\n REFUSED: this is a linked git worktree. These manifests reach sibling"
);
eprintln!(" repositories by relative path, and cargo resolves those against the");
eprintln!(" MANIFEST, so from here they point beside this worktree instead of");
eprintln!(" beside the main checkout at {}.", main.display());
eprintln!(" Reporting a pass would cover fewer packages than the same command in");
eprintln!(" the main checkout, with nothing in the output saying so.");
eprintln!(" Run `cargo xtask fmt` in {}.", main.display());
false
}
None => {
eprintln!("\n This is the main checkout and those siblings are not beside it, so");
eprintln!(" these packages cannot be checked on this machine at all. Reported");
eprintln!(" rather than counted as formatted.");
true
}
}
};
match mode {
Mode::Check if !unformatted.is_empty() => {
eprintln!(
"\nxtask fmt: NOT FORMATTED, {} of {} package(s):",
unformatted.len(),
manifests.len()
);
for b in &unformatted {
eprintln!(" {b}");
}
eprintln!("\n Fix with `cargo xtask fmt --write`.");
eprintln!(" Do NOT reach for `cargo fmt --all`: it rewrites five other");
eprintln!(" repositories and still misses three trees in this one. See the");
eprintln!(" module docs of xtask/src/fmt.rs for the measurement.");
false
}
Mode::Check => {
eprintln!(
"xtask fmt: {} of {} package(s) formatted",
manifests.len() - unreadable.len(),
manifests.len()
);
unreadable_ok
}
Mode::Write => {
eprintln!(
"xtask fmt: wrote {} of {} package(s)",
manifests.len() - unreadable.len(),
manifests.len()
);
unformatted.is_empty() && unreadable_ok
}
}
}
#[cfg(test)]
mod tests {
use super::*;
/// **The defect.** `cargo metadata` failing rendered as "NOT FORMATTED",
/// and the fix it suggested writes nothing.
///
/// Seen red: with `classify` returning `Verdict::NotFormatted`
/// unconditionally — the behaviour this replaced — the first two cases
/// fail, naming the stderr they were handed.
#[test]
fn a_manifest_cargo_cannot_load_is_not_a_formatting_verdict() {
let metadata = "`cargo metadata` exited with an error: error: failed to load manifest \
for dependency `draupnir`\n\nCaused by:\n failed to read \
/home/x/draupnir/Cargo.toml\n";
assert_eq!(
classify(metadata),
Verdict::Unreadable("failed to read /home/x/draupnir/Cargo.toml".to_owned()),
"a manifest cargo could not load was reported as unformatted code; rustfmt never \
looked at it, and `--write` would fail in exactly the same place"
);
let direct = "error: failed to load manifest for dependency `znippy-common`\n";
assert!(
matches!(classify(direct), Verdict::Unreadable(_)),
"cargo-fmt also reports the load failure without the `cargo metadata` preamble"
);
// …and a real formatting failure still is one. Without this arm,
// `classify` returning `Unreadable` for everything would pass.
assert_eq!(
classify("Diff in /home/x/gunnar/src/lib.rs at line 12:\n"),
Verdict::NotFormatted,
"rustfmt's own diff must stay a formatting verdict"
);
assert_eq!(
classify(""),
Verdict::NotFormatted,
"an unrecognised failure falls back to NOT FORMATTED: a false red, never a false green"
);
}
/// A scratch tree of packages, removed on drop.
struct Tree(PathBuf);
impl Tree {
fn new(name: &str) -> Tree {
let root =
std::env::temp_dir().join(format!("gunnar-fmt-{name}-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&root);
std::fs::create_dir_all(&root).unwrap();
Tree(root)
}
/// A package `name` under `dir`, with `lib` as its src/lib.rs.
fn package(&self, dir: &str, name: &str, deps: &str, lib: &str) -> PathBuf {
let at = self.0.join(dir);
std::fs::create_dir_all(at.join("src")).unwrap();
std::fs::write(
at.join("Cargo.toml"),
format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n[workspace]\n\n[dependencies]\n{deps}"),
)
.unwrap();
std::fs::write(at.join("src/lib.rs"), lib).unwrap();
at.join("src/lib.rs")
}
}
impl Drop for Tree {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
const TIDY: &str = "pub fn one() -> u32 {\n 1\n}\n";
const MESSY: &str = "pub fn one( )->u32{1}\n";
fn git(dir: &Path, args: &[&str]) {
let ok = Command::new("git")
.current_dir(dir)
.args(["-c", "user.name=t", "-c", "user.email=t@mock.invalid"])
.args(args)
.output()
.unwrap()
.status
.success();
assert!(ok, "git {args:?} in {}", dir.display());
}
/// The scan finds packages wherever they sit, and never in `target/`, a
/// fixture tree or a virtual manifest.
#[test]
fn the_packages_are_every_manifest_with_a_package_table() {
let t = Tree::new("scan");
t.package("a", "a", "", TIDY);
t.package("nested/b", "b", "", TIDY);
t.package("target/c", "c", "", TIDY);
t.package("xtask/tests/fixtures/d", "d", "", TIDY);
std::fs::write(t.0.join("Cargo.toml"), "[workspace]\nmembers = []\n").unwrap();
let found: Vec<String> = package_manifests(&t.0)
.iter()
.map(|p| p.strip_prefix(&t.0).unwrap().display().to_string())
.collect();
assert_eq!(found, ["a/Cargo.toml", "nested/b/Cargo.toml"]);
}
/// A tree with no package is a failure, never an empty success.
#[test]
fn a_tree_with_no_package_fails() {
let t = Tree::new("empty");
assert!(!run(&t.0, Mode::Check));
}
/// Check names an unformatted package and changes nothing; write formats it;
/// the check after the write passes.
#[test]
fn check_refuses_unformatted_code_and_write_fixes_it() {
let t = Tree::new("messy");
t.package("tidy", "tidy", "", TIDY);
let messy = t.package("messy", "messy", "", MESSY);
assert!(
!run(&t.0, Mode::Check),
"an unformatted package passed the check"
);
assert_eq!(
std::fs::read_to_string(&messy).unwrap(),
MESSY,
"the check wrote"
);
assert!(run(&t.0, Mode::Write));
assert_eq!(std::fs::read_to_string(&messy).unwrap(), TIDY);
assert!(run(&t.0, Mode::Check));
}
/// A package whose path dependency is not beside the checkout is reported as
/// unreadable: in a main checkout that is not a failure, in a linked
/// worktree it is refused.
#[test]
fn an_unreadable_package_passes_a_main_checkout_and_is_refused_in_a_worktree() {
let t = Tree::new("ghost");
t.package("tidy", "tidy", "", TIDY);
t.package(
"lonely",
"lonely",
"ghost = { path = \"../ghost\" }\n",
TIDY,
);
assert!(
run(&t.0, Mode::Check),
"the main checkout cannot check what is not there"
);
assert_eq!(
crate::worktree::main_worktree(&t.0),
None,
"not a git checkout at all"
);
git(&t.0, &["init", "-q"]);
git(&t.0, &["add", "-A"]);
git(&t.0, &["commit", "-q", "-m", "packages"]);
let linked = t.0.join("linked");
git(
&t.0,
&[
"worktree",
"add",
"-q",
"--detach",
linked.to_str().unwrap(),
],
);
assert_eq!(
crate::worktree::main_worktree(&t.0),
None,
"the main checkout is not linked"
);
assert_eq!(
crate::worktree::main_worktree(&linked).map(|p| std::fs::canonicalize(p).unwrap()),
Some(std::fs::canonicalize(&t.0).unwrap())
);
assert!(
!run(&linked, Mode::Check),
"a linked worktree checked fewer packages and passed"
);
}
}