kernel, patch, doctor, config: fill in the rest of Now and Next

kernel quest reads bugzilla's REST API and picks one bug through a seeded
xorshift, so a pick is reproducible in a test and varied in a person's hands.
kernel test builds under spawn_blocking and boots under qemu with the serial
console streamed line by line, and it reports what qemu did instead of
flattening a failed boot into a success.

patch create writes a whole series when --range is given. git format-patch
does the naming and the numbering because that is already its job, the cover
letter comes from --cover-letter, and a series.toml beside the patches records
the range with both ends as hashes, the revision, the files and the blurb.
patch submit takes a directory or several files and sends them in one
git send-email invocation, and patch update re-rolls a directory through
--reroll-count, deleting the superseded revision only once the new one is
complete.

doctor runs its checks in the order a first run hits them, prints one line and
a fix hint for each, and names the first hard failure in the summary. A missing
qemu or a missing mail route warns and keeps exit 0, because --dry-run is where
the loop actually stops on a machine without an SMTP route.

~/.config/spectral/config.toml holds the tree path, the patch directory,
always-CC addresses and an identity, with the environment winning over the file
and the file over the built-in default. spectral init verifies or clones a tree
and writes it once: a second run with the same tree says so and writes nothing,
and a run that would change an existing answer is refused until --force.

tests/ grew a fixture for each of those. The harness controls PATH and git's
configuration, and its PATH can hold a git that answers send-email either way,
so a host without that support still passes rather than deciding the result.

Two review lanes ran over this work and this commit carries their fixes. A
re-roll regenerated base..HEAD, so a commit that landed after create would have
silently joined the set and been mailed: the range is recorded now and a
re-roll regenerates exactly it. A patch file that was already gone made update
fail forever while the sidecar described a revision that was not on disk: a
missing file is the state the re-roll wanted. An empty blurb aborted after the
files were written: it is checked before anything is generated. A tree that is
not a git repository was reported as a missing revision. init cloned before it
refused. The bugzilla request had no timeout, a signal-ended child printed the
bare word "signal", CommandSpawn dropped the io reason, and the printed
send-email line quoted only arguments with spaces, so --in-reply-to <id@host>
pasted as a redirection.

Checked with cargo fmt --all -- --check, cargo clippy --all-targets --locked --
-D warnings, cargo test --locked (96 passed, 4 ignored) and cargo doc --no-deps.
The four ignored tests want the host's own checkpatch, get_maintainer and
bugzilla, and all four pass when asked for.
This commit is contained in:
2026-09-16 21:52:05 -04:00
parent b74dafb475
commit 1c668c3c1e
19 changed files with 3432 additions and 155 deletions
+36 -10
View File
@@ -30,6 +30,26 @@ pub enum Command {
#[command(subcommand)]
command: PatchCommand,
},
/// Check the tree, the scripts, the mail setup and the paths
Doctor,
/// Point the config at a kernel tree, verifying or cloning one
Init(InitArgs),
}
#[derive(Debug, Args)]
pub struct InitArgs {
/// A kernel tree you already have
#[arg(long, value_name = "PATH")]
pub tree: Option<PathBuf>,
/// Clone a tree from here instead
#[arg(long, value_name = "URL")]
pub clone: Option<String>,
/// Clone only this many commits deep
#[arg(long = "depth", value_name = "N", requires = "clone")]
pub depth: Option<u32>,
/// Overwrite a config that already points somewhere else
#[arg(long)]
pub force: bool,
}
#[derive(Debug, Subcommand)]
@@ -71,11 +91,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 a .patch file
/// Write the change out as a patch file, or as a series for a range
Create(CreateArgs),
/// Send a patch, with To/CC taken from get_maintainer.pl
/// Send a patch or a series, with To/CC taken from get_maintainer.pl
Submit(SubmitArgs),
/// Re-roll a patch as vN, renaming the file to match
/// Re-roll a patch, or a whole series, as vN
Update(UpdateArgs),
}
@@ -114,22 +134,28 @@ pub struct CommitArgs {
#[derive(Debug, Args)]
pub struct CreateArgs {
/// Patch name; `.patch` is appended if you leave it off
/// Patch name, or the series directory name when --range is given
#[arg(value_name = "NAME")]
pub name: String,
pub name: Option<String>,
/// Branch or revision to diff against
#[arg(long, value_name = "REV", default_value = "master")]
pub base: String,
/// Directory to write the patch into; defaults to the patch dir
/// Directory to write into; defaults to the patch dir
#[arg(long, value_name = "DIR")]
pub out: Option<PathBuf>,
/// Write a series for these commits, e.g. HEAD~3..HEAD
#[arg(long, value_name = "RANGE")]
pub range: Option<String>,
/// Cover letter text; its first line becomes the subject
#[arg(long, value_name = "BLURB")]
pub cover_letter: Option<String>,
}
#[derive(Debug, Args)]
pub struct SubmitArgs {
/// Patch file to send
#[arg(value_name = "PATCH")]
pub patch: PathBuf,
/// Patch file to send, or a directory holding a series
#[arg(value_name = "PATCH", num_args = 1..)]
pub patches: Vec<PathBuf>,
/// Print the recipients and command instead of sending
#[arg(long)]
pub dry_run: bool,
@@ -143,7 +169,7 @@ pub struct SubmitArgs {
#[derive(Debug, Args)]
pub struct UpdateArgs {
/// Patch file to re-roll
/// Patch file to re-roll, or the directory holding a series
#[arg(value_name = "PATCH")]
pub patch: PathBuf,
/// Revision to re-roll as; defaults to one past the file's current vN
+305 -16
View File
@@ -1,48 +1,130 @@
//! Where the kernel tree and the patches live.
//! Where the kernel tree and the patches live, and the file that can say so.
//!
//! 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
//! Everything else asks this module for paths, so the config file and
//! `spectral init` touch nothing else. The precedence is the one the roadmap
//! promised: the environment wins over the file, and the file wins over the
//! built-in default.
use std::env;
use std::path::{Path, PathBuf};
use std::process::Command;
use crate::error::{Error, Result};
use serde::{Deserialize, Serialize};
use crate::cli::InitArgs;
use crate::error::{Error, Result, exit_code};
/// Environment variable that points at the kernel checkout.
pub const ENV_KERNEL_TREE: &str = "SPECTRAL_KERNEL";
/// Tree used when that variable is unset, relative to `$HOME`.
/// Environment variable that points at the patch directory.
pub const ENV_PATCH_DIR: &str = "SPECTRAL_PATCH_DIR";
/// Tree used when nothing else says otherwise, relative to `$HOME`.
const DEFAULT_KERNEL_TREE: &str = ".spectral/linux";
/// Where written patches land, relative to `$HOME`.
/// Where written patches land by default, relative to `$HOME`.
const DEFAULT_PATCH_DIR: &str = ".spectral/patches";
/// The file, relative to the config directory.
const CONFIG_FILE: &str = "spectral/config.toml";
/// What the config file holds.
///
/// Unknown keys are refused rather than ignored: a mistyped key that silently
/// does nothing is worse than one that stops the run and says so.
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct ConfigFile {
/// Where the kernel tree is.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub kernel_tree: Option<PathBuf>,
/// Where written patches go.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub patch_dir: Option<PathBuf>,
/// Addresses every patch is copied to, on top of what get_maintainer says.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub always_cc: Vec<String>,
/// Who a patch is sent as.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub send_email: Option<SendEmail>,
}
/// The `[send_email]` table.
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct SendEmail {
/// `git send-email --from`, when it should differ from git's own identity.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub from: Option<String>,
}
/// Where a resolved path came from.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Origin {
/// One of the two environment variables.
Environment(&'static str),
/// The config file.
File,
/// The built-in default, relative to `$HOME`.
Default,
}
impl Origin {
/// How the report names it.
#[must_use]
pub fn describe(self) -> String {
match self {
Self::Environment(variable) => format!("from ${variable}"),
Self::File => "from the config file".to_owned(),
Self::Default => "the built-in default".to_owned(),
}
}
}
#[derive(Debug, Clone)]
pub struct Config {
kernel_tree: PathBuf,
patch_dir: PathBuf,
tree_origin: Origin,
patch_origin: Origin,
file: ConfigFile,
file_path: PathBuf,
}
impl Config {
/// Resolve the tree from `$SPECTRAL_KERNEL`, falling back to
/// `~/.spectral/linux`.
/// Resolve both paths: environment first, then the config file, then the
/// default under `$HOME`.
///
/// 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.
pub fn load() -> Result<Self> {
let home = home_dir();
let kernel_tree = env::var_os(ENV_KERNEL_TREE)
.map_or_else(|| home.join(DEFAULT_KERNEL_TREE), PathBuf::from);
let file_path = config_path(&home);
let file = read_file(&file_path)?;
let (kernel_tree, tree_origin) = resolve(
ENV_KERNEL_TREE,
file.kernel_tree.clone(),
home.join(DEFAULT_KERNEL_TREE),
)?;
let (patch_dir, patch_origin) = resolve(
ENV_PATCH_DIR,
file.patch_dir.clone(),
home.join(DEFAULT_PATCH_DIR),
)?;
Ok(Self {
kernel_tree,
patch_dir: home.join(DEFAULT_PATCH_DIR),
patch_dir,
tree_origin,
patch_origin,
file,
file_path,
})
}
@@ -56,6 +138,42 @@ impl Config {
&self.patch_dir
}
/// Where the tree path came from, for `doctor` to print.
///
/// A path that was resolved for the user and silently turns out to be the
/// wrong one is a worse report than one that says which knob set it.
#[must_use]
pub fn kernel_tree_origin(&self) -> String {
self.tree_origin.describe()
}
/// Where the patch directory came from, for `doctor` to print.
#[must_use]
pub fn patch_dir_origin(&self) -> String {
self.patch_origin.describe()
}
/// The identity a patch is sent as, when the config file names one.
#[must_use]
pub fn send_email_from(&self) -> Option<String> {
self.file
.send_email
.as_ref()
.and_then(|send_email| send_email.from.clone())
}
/// Addresses every patch is copied to.
#[must_use]
pub fn always_cc(&self) -> &[String] {
&self.file.always_cc
}
/// The config file this run read, whether or not it was there.
#[must_use]
pub fn file_path(&self) -> &Path {
&self.file_path
}
/// 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() {
@@ -73,6 +191,177 @@ impl Config {
}
}
/// What `spectral init` did.
#[derive(Debug, Clone)]
pub struct Init {
/// The file that was written, or that already said this.
pub config_file: PathBuf,
/// The tree the config now names.
pub kernel_tree: PathBuf,
/// Whether anything was written.
pub wrote: bool,
}
/// `spectral init`: point the config at a tree, verifying or cloning one.
///
/// Idempotent on purpose. A second run that would change nothing says so and
/// writes nothing; a run that would change an existing answer is refused until
/// `--force` says it meant it.
pub fn init(args: &InitArgs) -> Result<Init> {
let home = home_dir();
let config_file = config_path(&home);
let current = read_file(&config_file)?;
let tree = match (&args.tree, &args.clone) {
// The destination is named first: the refusal below has to happen
// before a clone runs, not after one has already landed.
(tree, Some(_)) => tree
.clone()
.unwrap_or_else(|| home.join(DEFAULT_KERNEL_TREE)),
(Some(tree), None) => tree.clone(),
(None, None) => Config::load()?.kernel_tree().to_path_buf(),
};
let mut next = current.clone();
next.kernel_tree = Some(tree.clone());
let changed = next != current;
if changed && config_file.is_file() && !args.force {
return Err(Error::ConfigSaysSomethingElse { path: config_file });
}
if let Some(url) = &args.clone {
clone_tree(url, &tree, args.depth)?;
}
verify_tree(&tree)?;
if changed {
write_file(&config_file, &next)?;
}
Ok(Init {
config_file,
kernel_tree: tree,
wrote: changed,
})
}
/// Clone `url` into `into`.
///
/// A tree already sitting there is left alone: `init` run twice should not
/// re-clone anything.
fn clone_tree(url: &str, into: &Path, depth: Option<u32>) -> Result<()> {
if into.join("scripts/checkpatch.pl").is_file() {
return Ok(());
}
if let Some(parent) = into.parent() {
std::fs::create_dir_all(parent)?;
}
let mut args = vec!["clone".to_owned()];
if let Some(depth) = depth {
args.push("--depth".to_owned());
args.push(depth.to_string());
}
args.push(url.to_owned());
args.push(into.display().to_string());
let output =
Command::new("git")
.args(&args)
.output()
.map_err(|source| Error::CommandSpawn {
program: "git clone".to_owned(),
source,
})?;
if !output.status.success() {
return Err(Error::ExternalCommand {
program: "git".to_owned(),
args: args.join(" "),
code: exit_code(&output.status),
stderr: String::from_utf8_lossy(&output.stderr)
.trim_end()
.to_owned(),
});
}
Ok(())
}
/// Check that a path really is a kernel source tree.
fn verify_tree(tree: &Path) -> Result<()> {
if tree.join("scripts/checkpatch.pl").is_file() {
Ok(())
} else if tree.exists() {
Err(Error::NotAKernelTree {
path: tree.to_path_buf(),
})
} else {
Err(Error::KernelTreeMissing {
path: tree.to_path_buf(),
env: ENV_KERNEL_TREE,
})
}
}
/// Read the config file, or hand back an empty one when it is not there.
fn read_file(path: &Path) -> Result<ConfigFile> {
let text = match std::fs::read_to_string(path) {
Ok(text) => text,
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
return Ok(ConfigFile::default());
}
Err(error) => return Err(Error::Io(error)),
};
toml::from_str(&text).map_err(|source| Error::ConfigUnreadable {
path: path.to_path_buf(),
source,
})
}
/// Write the config file, making its directory first.
fn write_file(path: &Path, file: &ConfigFile) -> Result<()> {
let text = toml::to_string(file).map_err(|source| Error::ConfigUnwritable { source })?;
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
}
std::fs::write(path, text)?;
Ok(())
}
/// Where the config file lives: `$XDG_CONFIG_HOME/spectral/config.toml`, or
/// `~/.config/spectral/config.toml` when that variable is unset.
#[must_use]
pub fn config_path(home: &Path) -> PathBuf {
env::var_os("XDG_CONFIG_HOME")
.map_or_else(|| home.join(".config"), PathBuf::from)
.join(CONFIG_FILE)
}
/// Pick one path out of the environment, the file, and the default.
///
/// A relative path in the file is refused rather than resolved against
/// whichever directory the command happened to run in.
fn resolve(
variable: &'static str,
from_file: Option<PathBuf>,
fallback: PathBuf,
) -> Result<(PathBuf, Origin)> {
if let Some(value) = env::var_os(variable) {
return Ok((PathBuf::from(value), Origin::Environment(variable)));
}
if let Some(path) = from_file {
if !path.is_absolute() {
return Err(Error::ConfigPathRelative { variable, path });
}
return Ok((path, Origin::File));
}
Ok((fallback, Origin::Default))
}
fn home_dir() -> PathBuf {
env::var_os("HOME").map_or_else(|| PathBuf::from("."), PathBuf::from)
}
+436
View File
@@ -0,0 +1,436 @@
//! `spectral doctor`: what is missing before the loop can run.
//!
//! The checks run in the order a first attempt hits them, and the summary names
//! the first hard failure, because that is the one worth fixing first. A
//! warning is something spectral works around or reports without stopping: a
//! host with no SMTP route still exits 0, because `--dry-run` is what the
//! README tells a reader to trust.
use std::io::Write as _;
use std::path::{Path, PathBuf};
use std::process::Command;
use crate::config::Config;
/// How one check ended.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Verdict {
/// Anything that needs it can go ahead.
Ok,
/// Worth knowing, not worth stopping for.
Warn,
/// Something spectral cannot work without.
Fail,
/// Not looked at, because something it depends on failed first.
Skipped,
}
impl Verdict {
fn label(self) -> &'static str {
match self {
Self::Ok => "ok",
Self::Warn => "warn",
Self::Fail => "fail",
Self::Skipped => "skip",
}
}
fn is_failure(self) -> bool {
matches!(self, Self::Fail)
}
}
/// One check and what it found.
#[derive(Debug, Clone)]
pub struct Check {
/// Short name, so the summary can name the one that failed.
pub name: &'static str,
pub verdict: Verdict,
pub detail: String,
/// What to do about it, printed under the line.
pub fix: Option<String>,
}
impl Check {
fn ok(name: &'static str, detail: impl Into<String>) -> Self {
Self {
name,
verdict: Verdict::Ok,
detail: detail.into(),
fix: None,
}
}
fn warn(name: &'static str, detail: impl Into<String>, fix: impl Into<String>) -> Self {
Self {
name,
verdict: Verdict::Warn,
detail: detail.into(),
fix: Some(fix.into()),
}
}
fn fail(name: &'static str, detail: impl Into<String>, fix: impl Into<String>) -> Self {
Self {
name,
verdict: Verdict::Fail,
detail: detail.into(),
fix: Some(fix.into()),
}
}
fn skipped(name: &'static str, detail: impl Into<String>) -> Self {
Self {
name,
verdict: Verdict::Skipped,
detail: detail.into(),
fix: None,
}
}
}
/// Everything the checks found.
#[derive(Debug, Clone)]
pub struct Report {
pub checks: Vec<Check>,
}
impl Report {
/// Whether anything hard failed. Warnings do not count.
#[must_use]
pub fn ok(&self) -> bool {
!self.checks.iter().any(|check| check.verdict.is_failure())
}
/// The first hard failure, which is the one worth naming.
#[must_use]
pub fn first_failure(&self) -> Option<&Check> {
self.checks.iter().find(|check| check.verdict.is_failure())
}
fn count(&self, verdict: Verdict) -> usize {
self.checks
.iter()
.filter(|check| check.verdict == verdict)
.count()
}
/// The report as `main` prints it: one line per check, the fix under it,
/// and a summary that names the first failure.
#[must_use]
pub fn render(&self) -> String {
let mut out = String::new();
for check in &self.checks {
out.push_str(&format!(
"{:<4} {:<23} {}\n",
check.verdict.label(),
check.name,
check.detail
));
if let Some(fix) = &check.fix {
out.push_str(&format!(" {:<23} -> {fix}\n", ""));
}
}
let failures = self.count(Verdict::Fail);
let warnings = self.count(Verdict::Warn);
match self.first_failure() {
Some(first) => out.push_str(&format!(
"doctor: {failures} failed, {warnings} warned; first failure: {}\n",
first.name
)),
None if warnings > 0 => {
out.push_str(&format!("doctor: nothing failed, {warnings} warned\n"));
}
None => out.push_str("doctor: everything checked out\n"),
}
out
}
}
/// Run every check, in the order a first attempt hits them.
#[must_use]
pub fn run(config: &Config) -> Report {
let tree = config.require_kernel_tree().ok().map(Path::to_path_buf);
let checks = vec![
tree_path(config),
tree_shape(config),
tree_file(
tree.as_deref(),
"scripts/checkpatch.pl",
"checkpatch.pl",
"the only thing that decides kernel style",
),
tree_file(
tree.as_deref(),
"scripts/get_maintainer.pl",
"get_maintainer.pl",
"the only thing that knows who to mail",
),
tree_file(
tree.as_deref(),
"MAINTAINERS",
"MAINTAINERS",
"the maintainer map the script reads",
),
git_runs(),
send_email_invocable(),
sender_identity(config, tree.as_deref()),
mail_route(tree.as_deref()),
qemu_on_path(),
patch_dir_writable(config),
config_file(config),
];
Report { checks }
}
/// Whether a config file is there, and where it is.
///
/// A warning: everything works without one, and the paths in the report above
/// say where the settings came from. The file is what `spectral init` writes,
/// so a missing one is a pointer rather than a problem.
fn config_file(config: &Config) -> Check {
let path = config.file_path();
if path.is_file() {
Check::ok("config file", path.display().to_string())
} else {
Check::warn(
"config file",
format!(
"no config file at {}; the paths above are defaults",
path.display()
),
"run `spectral init --tree <path>` to write one",
)
}
}
/// Where spectral thinks the tree is, and where it got that from.
fn tree_path(config: &Config) -> Check {
let path = config.kernel_tree();
if path.is_dir() {
Check::ok(
"kernel tree",
format!("{} ({})", path.display(), config.kernel_tree_origin()),
)
} else {
Check::fail(
"kernel tree",
format!("{} ({})", path.display(), config.kernel_tree_origin()),
"clone a tree there, or point the path at one you have",
)
}
}
/// Whether the path really is a kernel source tree.
fn tree_shape(config: &Config) -> Check {
if !config.kernel_tree().exists() {
return Check::skipped(
"kernel tree shape",
"not looked at: there is nothing at that path",
);
}
match config.require_kernel_tree() {
Ok(_) => Check::ok("kernel tree shape", "scripts/checkpatch.pl is there"),
Err(error) => Check::fail(
"kernel tree shape",
error.to_string(),
"point the tree path at a kernel checkout",
),
}
}
/// One file inside the tree.
fn tree_file(tree: Option<&Path>, rel: &'static str, name: &'static str, why: &str) -> Check {
let Some(tree) = tree else {
return Check::skipped(name, "not looked at: there is no tree");
};
let path = tree.join(rel);
if path.is_file() {
Check::ok(name, format!("{rel} ({why})"))
} else {
Check::fail(
name,
format!("no {rel} in {}", tree.display()),
"use a full kernel checkout, not a build directory",
)
}
}
/// Whether git runs at all.
fn git_runs() -> Check {
match Command::new("git").arg("--version").output() {
Ok(output) if output.status.success() => {
let version = String::from_utf8_lossy(&output.stdout).trim().to_owned();
Check::ok("git", version)
}
Ok(output) => Check::fail(
"git",
format!(
"`git --version` failed ({})",
crate::error::exit_code(&output.status)
),
"fix git on this machine before spectral can do anything",
),
Err(error) => Check::fail("git", format!("could not run git: {error}"), "install git"),
}
}
/// Whether `git send-email` is installed.
///
/// `--dump-aliases` is the cheapest call that proves the script runs: it reads
/// the same config send-email would and needs no patch and no mail host.
fn send_email_invocable() -> Check {
let output = Command::new("git")
.arg("send-email")
.arg("--dump-aliases")
.output();
match output {
Ok(output) if output.status.success() => Check::ok("git send-email", "runs"),
Ok(output) => Check::fail(
"git send-email",
format!(
"`git send-email --dump-aliases` failed ({}): {}",
crate::error::exit_code(&output.status),
String::from_utf8_lossy(&output.stderr).trim_end()
),
"install git's send-email support (on Arch: git is not enough on its own)",
),
Err(error) => Check::fail(
"git send-email",
format!("could not run git send-email: {error}"),
"install git",
),
}
}
/// Who a patch is sent as.
///
/// A hard failure: `git send-email` refuses a patch with no sender, and finding
/// that out at the end of the loop is the worst time.
fn sender_identity(config: &Config, tree: Option<&Path>) -> Check {
if let Some(from) = config.send_email_from() {
return Check::ok("sender identity", format!("{from} (from the config file)"));
}
let Some(tree) = tree else {
return Check::skipped("sender identity", "not looked at: there is no tree");
};
let email = git_config(tree, "user.email");
match email {
Some(email) if !email.trim().is_empty() => Check::ok("sender identity", email),
_ => Check::fail(
"sender identity",
"git has no user.email in this tree",
"set `git config --global user.email [email protected]`, or an identity in the config file",
),
}
}
/// Whether anything would carry mail out.
///
/// A warning, not a failure: this host has no `sendemail.*` config at all and
/// still runs `git send-email --dry-run` to completion, which is the half of
/// the loop spectral itself relies on.
fn mail_route(tree: Option<&Path>) -> Check {
let Some(tree) = tree else {
return Check::skipped("mail route", "not looked at: there is no tree");
};
let configured = Command::new("git")
.arg("-C")
.arg(tree)
.args(["config", "--get-regexp", r"^sendemail\."])
.output();
match configured {
Ok(output) if !String::from_utf8_lossy(&output.stdout).trim().is_empty() => {
Check::ok("mail route", "sendemail.* is configured")
}
_ => Check::warn(
"mail route",
"no sendemail.* config: --dry-run works, a real send will not",
"configure `git send-email` before dropping --dry-run",
),
}
}
/// Whether qemu is there to boot with.
///
/// A warning: `kernel test` cannot run without it, and nothing else cares.
fn qemu_on_path() -> Check {
match Command::new("qemu-system-x86_64").arg("--version").output() {
Ok(output) if output.status.success() => {
let version = String::from_utf8_lossy(&output.stdout)
.lines()
.next()
.unwrap_or_default()
.trim()
.to_owned();
Check::ok("qemu", version)
}
_ => Check::warn(
"qemu",
"qemu-system-x86_64 is not on $PATH, so `kernel test` cannot boot anything",
"install qemu (qemu-system-x86 on Arch)",
),
}
}
/// Whether patches can be written where spectral wants them.
///
/// This is the one check with a side effect: it makes the directory when it is
/// missing, which is what the patch verbs would do anyway, and says which of
/// the two happened.
fn patch_dir_writable(config: &Config) -> Check {
let dir: PathBuf = config.patch_dir().to_path_buf();
let existed = dir.is_dir();
if let Err(error) = std::fs::create_dir_all(&dir) {
return Check::fail(
"patch directory",
format!("{} cannot be created: {error}", dir.display()),
"point the patch directory somewhere writable",
);
}
let probe = dir.join(format!(".spectral-doctor-{}", std::process::id()));
let wrote = std::fs::File::create(&probe).and_then(|mut file| file.write_all(b"probe\n"));
let _ = std::fs::remove_file(&probe);
let state = if existed { "exists" } else { "created" };
match wrote {
Ok(()) => Check::ok(
"patch directory",
format!("{} ({state}, {})", dir.display(), config.patch_dir_origin()),
),
Err(error) => Check::fail(
"patch directory",
format!("{} is not writable: {error}", dir.display()),
"point the patch directory somewhere writable",
),
}
}
/// One key out of git's configuration, or nothing when it is unset.
fn git_config(tree: &Path, key: &str) -> Option<String> {
let output = Command::new("git")
.arg("-C")
.arg(tree)
.args(["config", key])
.output()
.ok()?;
if output.status.success() {
Some(
String::from_utf8_lossy(&output.stdout)
.trim_end()
.to_owned(),
)
} else {
None
}
}
+138 -7
View File
@@ -10,10 +10,24 @@ use std::process::ExitStatus;
pub type Result<T, E = Error> = std::result::Result<T, E>;
/// How a failed process is described in [`Error::ExternalCommand`].
///
/// A process killed by a signal has no exit code, and the bare word "signal"
/// throws away which one: a SIGTERM from a `timeout` and a SIGSEGV from the
/// program under test are different answers to why it stopped.
pub(crate) fn exit_code(status: &ExitStatus) -> String {
status
.code()
.map_or_else(|| "signal".to_owned(), |code| code.to_string())
if let Some(code) = status.code() {
return code.to_string();
}
#[cfg(unix)]
{
use std::os::unix::process::ExitStatusExt as _;
if let Some(signal) = status.signal() {
return format!("signal {signal}");
}
}
"a signal".to_owned()
}
#[derive(Debug, thiserror::Error)]
@@ -36,13 +50,21 @@ pub enum Error {
},
/// An external tool could not even be started.
#[error("could not run `{program}`")]
#[error("could not run `{program}`: {source}")]
CommandSpawn {
program: String,
#[source]
source: std::io::Error,
},
/// A patch file the user named could not be read.
#[error("`{path}` could not be read: {source}")]
PatchUnreadable {
path: PathBuf,
#[source]
source: std::io::Error,
},
/// A patch file the user named does not exist.
#[error("no patch named `{name}` in `{dir}`")]
PatchNotFound { name: String, dir: PathBuf },
@@ -57,7 +79,7 @@ pub enum Error {
/// One patch file was asked to carry a whole set.
#[error(
"`{base}..HEAD` holds {count} commits; one patch file cannot carry a set (patch series support is not built yet)"
"`{base}..HEAD` holds {count} commits; one file cannot carry a set, so write it as a series with `--range {base}..HEAD`"
)]
RangeHoldsSeveralCommits { base: String, count: usize },
@@ -75,11 +97,120 @@ pub enum Error {
#[error("`{path}` already exists; delete it or pick another revision")]
RerollWouldOverwrite { path: PathBuf },
/// Bugzilla answered, but with something that is not a bug list.
#[error("bugzilla answered with something that is not a bug list: {source}")]
BugzillaNotABugList {
#[source]
source: serde_json::Error,
},
/// Bugzilla answered with an empty set of open bugs.
#[error("bugzilla reports no open bugs at all; try again later")]
BugzillaEmpty,
/// `--filter` matched nothing, which is a dead end rather than a pick.
#[error("no open bug matched `{filter}`; drop --filter or try another word")]
NoQuestCandidates { filter: String },
/// The tree built, or was expected to have built, and holds no image.
#[error("no bootable image at `{path}`; build the tree first or drop --no-build")]
ImageMissing { path: PathBuf },
/// `make` ran and failed.
#[error("`make` failed (exit {code}); the tree did not build")]
BuildFailed { code: String },
/// The thread the build was moved onto did not come back.
#[error("the build stopped before it finished: {source}")]
BuildTask {
#[source]
source: tokio::task::JoinError,
},
/// `patch create` was given neither a name nor a range.
#[error("`patch create` wants a NAME, or --range for a series")]
CreateNeedsName,
/// `--range` was given something that is not a range.
#[error("`{range}` is not a range; `--range` wants `<base>..<head>`, e.g. HEAD~3..HEAD")]
RangeNotARange { range: String },
/// `--cover-letter` without a series to put it in.
#[error("`--cover-letter` belongs to a series; add `--range <base>..<head>`")]
CoverLetterNeedsSeries,
/// A blurb with nothing in it would leave the template subject behind.
#[error("the cover letter blurb is empty; give it a first line, which becomes the subject")]
CoverLetterBlurbEmpty,
/// format-patch wrote no cover letter where one was expected.
#[error("no cover letter was written into `{dir}`; a cover letter was asked for")]
CoverLetterMissing { dir: PathBuf },
/// A directory was used as a series but carries no sidecar.
#[error("`{path}` holds no series.toml; create the series with `patch create --range`")]
SeriesManifestMissing { path: PathBuf },
/// The sidecar is there and cannot be read.
#[error("`{path}` is not a readable series manifest: {source}")]
SeriesManifestUnreadable {
path: PathBuf,
#[source]
source: toml::de::Error,
},
/// The sidecar could not be written.
#[error("the series manifest could not be written: {source}")]
SeriesManifestUnwritable {
#[source]
source: toml::ser::Error,
},
/// A superseded revision would not go, so the directory holds two of them.
#[error(
"these files could not be removed: {names}; `submit` on the directory would send both revisions"
)]
StaleSeriesFiles { names: String },
/// A directory was handed to `submit` and holds no patches.
#[error("`{dir}` holds no `.patch` files")]
NoPatchesInDirectory { dir: PathBuf },
/// `git format-patch` was asked for a set and the range held nothing.
#[error("`{range}` holds no commits; a series needs at least one")]
RangeHoldsNoCommits { range: String },
/// The config file is there and is not readable as TOML.
#[error("`{path}` is not a readable config file: {source}")]
ConfigUnreadable {
path: PathBuf,
#[source]
source: toml::de::Error,
},
/// The config file could not be written.
#[error("the config file could not be written: {source}")]
ConfigUnwritable {
#[source]
source: toml::ser::Error,
},
/// A path in the config file would resolve against the working directory.
#[error(
"`{path}` in the config file is not an absolute path; ${variable} and the default are, so this one is refused"
)]
ConfigPathRelative {
variable: &'static str,
path: PathBuf,
},
/// The config already answers the question `init` was asked.
#[error("`{path}` already points somewhere else; pass --force to overwrite it")]
ConfigSaysSomethingElse { path: PathBuf },
#[error(transparent)]
Io(#[from] std::io::Error),
// Reached once `kernel quest` stops being a stub.
#[allow(dead_code)]
#[error(transparent)]
Http(#[from] reqwest::Error),
}
+60 -11
View File
@@ -67,9 +67,7 @@ impl Git {
program: "git".to_owned(),
args: format!("apply {}", args.join(" ")),
code: exit_code(&output.status),
stderr: String::from_utf8_lossy(&output.stderr)
.trim_end()
.to_owned(),
stderr: told_off(&output),
});
}
@@ -149,12 +147,49 @@ impl Git {
Ok(())
}
/// Resolve a revision to a commit hash.
/// Resolve a revision, or `None` when there is no such revision.
///
/// Callers want the hash: a branch name can move under a command that is
/// about to build a range out of it.
pub fn rev_parse(&self, rev: &str) -> Result<String> {
self.run(&["rev-parse", "--verify", "--quiet", rev])
/// A revision that does not resolve and a repository that is not there are
/// both failures to git, and they are not the same answer: git says so on
/// stderr when the repository is missing, and `--quiet` keeps a missing
/// revision silent. Reading that difference is what keeps "not a git
/// repository" from being reported as "no revision `master`".
pub fn rev_parse_optional(&self, rev: &str) -> Result<Option<String>> {
let args = ["rev-parse", "--verify", "--quiet", rev];
let output = self.attempt(&args)?;
if output.status.success() {
return Ok(Some(
String::from_utf8_lossy(&output.stdout)
.trim_end()
.to_owned(),
));
}
if output.stdout.is_empty() && output.stderr.is_empty() {
return Ok(None);
}
Err(Error::ExternalCommand {
program: "git".to_owned(),
args: args.join(" "),
code: exit_code(&output.status),
stderr: told_off(&output),
})
}
/// Run git and hand back what happened, failure included.
///
/// The one caller is [`Git::rev_parse_optional`], which has to look at a
/// non-zero exit rather than turn it into an error immediately.
fn attempt(&self, args: &[&str]) -> Result<std::process::Output> {
Command::new("git")
.arg("-C")
.arg(&self.repo)
.args(args)
.output()
.map_err(|source| Error::CommandSpawn {
program: format!("git {}", args.join(" ")),
source,
})
}
fn output(&self, args: &[&str]) -> Result<std::process::Output> {
@@ -173,12 +208,26 @@ impl Git {
program: "git".to_owned(),
args: args.join(" "),
code: exit_code(&output.status),
stderr: String::from_utf8_lossy(&output.stderr)
.trim_end()
.to_owned(),
stderr: told_off(&output),
});
}
Ok(output)
}
}
/// What a failed git said, wherever it said it.
///
/// git puts some failures on stdout rather than stderr. `git commit` with
/// nothing staged explains itself there, and a reader handed an empty message
/// cannot tell what went wrong.
fn told_off(output: &std::process::Output) -> String {
for stream in [&output.stderr, &output.stdout] {
let said = String::from_utf8_lossy(stream);
if !said.trim().is_empty() {
return said.trim_end().to_owned();
}
}
"nothing was captured".to_owned()
}
+223 -16
View File
@@ -1,13 +1,26 @@
//! `spectral kernel test`: build the tree, then boot it under qemu.
#![allow(dead_code)] // nothing is reachable until `run` stops being a stub
use std::path::{Path, PathBuf};
use std::process::ExitStatus;
use std::process::{Command, ExitStatus, Stdio};
use tokio::io::{AsyncBufReadExt as _, BufReader};
use tokio::process::Command as AsyncCommand;
use crate::cli::TestArgs;
use crate::config::Config;
use crate::error::Result;
use crate::error::{Error, Result, exit_code};
/// Where an x86 kernel leaves the image qemu boots.
const IMAGE: &str = "arch/x86/boot/bzImage";
/// The console the kernel is told to use.
///
/// It is the one that comes back on a terminal, which is the difference
/// between a boot hang someone can read and one they cannot.
const CONSOLE: &str = "console=ttyS0";
/// The emulator, by the name the README's prerequisites list.
const QEMU: &str = "qemu-system-x86_64";
/// One qemu run.
#[derive(Debug, Clone)]
@@ -20,25 +33,219 @@ pub struct Boot {
pub extra_args: Vec<String>,
}
/// What one `kernel test` did.
///
/// The status is carried rather than flattened into a `()`: a boot that failed
/// must not print the same thing as one that succeeded.
#[derive(Debug, Clone)]
pub struct Outcome {
/// The image qemu was given.
pub image: PathBuf,
/// How qemu ended. A kernel that panics and stays running ends in a signal
/// rather than in an exit code, and that is worth reporting as itself.
pub status: ExitStatus,
}
impl Outcome {
/// Whether qemu ended well.
#[must_use]
pub fn succeeded(&self) -> bool {
self.status.success()
}
}
/// Build the tree and hand back the path to the bootable image.
///
/// `make` inherits the terminal, because a kernel build that says nothing for
/// ten minutes is indistinguishable from one that has hung.
pub fn build(kernel_tree: &Path) -> Result<PathBuf> {
todo!(
"make -j$(nproc) in {}, then locate arch/x86/boot/bzImage",
kernel_tree.display()
)
let jobs = std::thread::available_parallelism().map_or(1, std::num::NonZero::get);
let status = Command::new("make")
.arg(format!("-j{jobs}"))
.current_dir(kernel_tree)
.status()
.map_err(|source| Error::CommandSpawn {
program: "make".to_owned(),
source,
})?;
if !status.success() {
return Err(Error::BuildFailed {
code: exit_code(&status),
});
}
existing_image(kernel_tree)
}
/// The qemu arguments for one boot, without the emulator itself.
///
/// Split out so the shape of the command line is a pure function a test can
/// assert on, which a spawned qemu is not.
fn boot_args(boot: &Boot) -> Vec<String> {
let mut args = vec![
"-kernel".to_owned(),
boot.kernel.display().to_string(),
"-append".to_owned(),
CONSOLE.to_owned(),
"-nographic".to_owned(),
];
if let Some(rootfs) = &boot.rootfs {
args.push("-initrd".to_owned());
args.push(rootfs.display().to_string());
}
args.extend(boot.extra_args.iter().cloned());
args
}
/// Boot an image and wait for qemu to exit.
///
/// The serial console is streamed line by line rather than buffered until qemu
/// exits: a boot that hangs halfway is exactly the case where the output that
/// arrived is the whole diagnosis.
pub async fn boot(kernel_tree: &Path, boot: &Boot) -> Result<ExitStatus> {
// TODO: qemu-system-x86_64 -kernel {boot.kernel} -append "console=ttyS0" \
// -nographic, with -initrd when `rootfs` is set and `extra_args` on
// the end. Stream serial output instead of buffering it.
let _ = (kernel_tree, boot);
todo!("boot under qemu and stream the console")
let mut child = AsyncCommand::new(QEMU)
.current_dir(kernel_tree)
.args(boot_args(boot))
.stdin(Stdio::null())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
// A qemu left behind by a failed run would hold the image and the
// terminal, so it goes when this does.
.kill_on_drop(true)
.spawn()
.map_err(|source| Error::CommandSpawn {
program: QEMU.to_owned(),
source,
})?;
let console = child
.stdout
.take()
.map(|pipe| tokio::spawn(drain(pipe, false)));
let complaints = child
.stderr
.take()
.map(|pipe| tokio::spawn(drain(pipe, true)));
let status = child.wait().await?;
if let Some(console) = console {
let _ = console.await;
}
if let Some(complaints) = complaints {
let _ = complaints.await;
}
Ok(status)
}
/// `spectral kernel test`: build unless told not to, then boot.
pub async fn run(config: &Config, args: TestArgs) -> Result<()> {
let _ = (config, args);
todo!("build + boot")
pub async fn run(config: &Config, args: TestArgs) -> Result<Outcome> {
let tree = config.require_kernel_tree()?.to_path_buf();
let image = if args.no_build {
existing_image(&tree)?
} else {
// A kernel build blocks for minutes, and blocking the async runtime
// for that long stalls everything else it is holding.
let building = tree.clone();
tokio::task::spawn_blocking(move || build(&building))
.await
.map_err(|source| Error::BuildTask { source })??
};
let planned = Boot {
kernel: image.clone(),
rootfs: args.rootfs,
extra_args: args.qemu_args,
};
let status = boot(&tree, &planned).await?;
Ok(Outcome { image, status })
}
/// The image a `--no-build` run was asked to boot.
fn existing_image(kernel_tree: &Path) -> Result<PathBuf> {
let image = kernel_tree.join(IMAGE);
if image.is_file() {
Ok(image)
} else {
Err(Error::ImageMissing { path: image })
}
}
/// Copy one of qemu's pipes to this process's own, line by line.
///
/// A read that fails ends the stream with a note rather than silently: the
/// last lines of a console are often the reason the run is being read at all.
async fn drain<R>(pipe: R, to_stderr: bool)
where
R: tokio::io::AsyncRead + Unpin,
{
let mut lines = BufReader::new(pipe).lines();
loop {
match lines.next_line().await {
Ok(Some(line)) if to_stderr => eprintln!("{line}"),
Ok(Some(line)) => println!("{line}"),
Ok(None) => break,
Err(error) => {
eprintln!("spectral: qemu's console stopped early: {error}");
break;
}
}
}
}
#[cfg(test)]
mod tests {
use super::*;
fn boot(rootfs: Option<&str>, extra_args: &[&str]) -> Boot {
Boot {
kernel: PathBuf::from("/tree/arch/x86/boot/bzImage"),
rootfs: rootfs.map(PathBuf::from),
extra_args: extra_args.iter().map(|arg| (*arg).to_owned()).collect(),
}
}
#[test]
fn a_boot_names_the_image_and_the_serial_console() {
assert_eq!(
boot_args(&boot(None, &[])),
vec![
"-kernel",
"/tree/arch/x86/boot/bzImage",
"-append",
"console=ttyS0",
"-nographic",
]
);
}
#[test]
fn a_rootfs_becomes_an_initrd_after_the_kernel() {
assert_eq!(
boot_args(&boot(Some("/img/initramfs.img"), &[])),
vec![
"-kernel",
"/tree/arch/x86/boot/bzImage",
"-append",
"console=ttyS0",
"-nographic",
"-initrd",
"/img/initramfs.img",
]
);
}
#[test]
fn extra_arguments_go_on_the_end_verbatim() {
assert_eq!(
boot_args(&boot(None, &["-smp", "2", "-m", "1G"]))[5..],
["-smp", "2", "-m", "1G"],
"the pass-through arguments are the caller's, in the order it gave them"
);
}
}
+339 -17
View File
@@ -2,14 +2,29 @@
//!
//! 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
//!
//! The data comes from bugzilla.kernel.org's REST API rather than its HTML:
//! `buglist.cgi` caps at 200 rows and would have to be paginated, and the CSV
//! export needs a quote-aware parser for nothing JSON does not already give.
use std::future::Future;
use std::time::{Duration, SystemTime, UNIX_EPOCH};
use serde::Deserialize;
use crate::cli::QuestArgs;
use crate::config::Config;
use crate::error::Result;
use crate::error::{Error, Result};
/// The bug fields the REST call asks for, which are the ones an [`Issue`]
/// needs. `limit=0` means everything rather than nothing.
const INCLUDE_FIELDS: &str = "id,summary,product,component";
/// How long bugzilla gets before the command gives up.
///
/// Without one, a route that drops packets rather than refusing the connection
/// hangs the command with nothing on screen and no way out but Ctrl-C.
const REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
/// One piece of work, normalised across whatever source it came from.
#[derive(Debug, Clone)]
@@ -23,6 +38,31 @@ pub struct Issue {
pub url: String,
}
impl Issue {
/// What `main` prints: the JSON form when `--json` asked for it, otherwise
/// the readable one.
#[must_use]
pub fn render(&self, json: bool) -> String {
if json {
let value = serde_json::json!({
"id": &self.id,
"title": &self.title,
"component": &self.component,
"url": &self.url,
});
if let Ok(rendered) = serde_json::to_string_pretty(&value) {
return rendered;
}
}
let component = self.component.as_deref().unwrap_or("unspecified");
format!(
"{} {}\ncomponent: {component}\n{}",
self.id, self.title, self.url
)
}
}
/// Anything spectral can pull open work from.
///
/// Written as `impl Future` rather than a bare `async fn` so the `Send` bound
@@ -46,29 +86,311 @@ impl Default for Bugzilla {
}
}
impl Bugzilla {
/// The REST call that returns every open bug and the four fields wanted.
fn open_bugs_url(&self) -> String {
format!(
"{}/rest/bug?status=NEW&limit=0&include_fields={INCLUDE_FIELDS}",
self.base_url.trim_end_matches('/')
)
}
/// Where a human reads the bug.
fn issue_url(&self, id: u64) -> String {
format!(
"{}/show_bug.cgi?id={id}",
self.base_url.trim_end_matches('/')
)
}
}
impl QuestSource for Bugzilla {
async fn fetch_open(&self) -> Result<Vec<Issue>> {
// TODO: GET `{base_url}/buglist.cgi` with status=NEW/ASSIGNED/REOPENED,
// walk the paginated result tables, and pull id + summary + product out
// of each row with `scraper`.
let _ = &self.base_url;
todo!("scrape open bugs from bugzilla.kernel.org")
let body = reqwest::Client::builder()
.timeout(REQUEST_TIMEOUT)
.build()?
.get(self.open_bugs_url())
.send()
.await?
.text()
.await?;
parse_open_bugs(self, &body)
}
}
/// What the REST call answered with, in the shape it answered with it.
#[derive(Debug, Deserialize)]
struct BugList {
bugs: Vec<Bug>,
}
/// One bug, with only the fields that were asked for.
#[derive(Debug, Deserialize)]
struct Bug {
id: u64,
summary: String,
product: Option<String>,
component: Option<String>,
}
/// Read a `/rest/bug` answer into issues.
///
/// Kept separate from the call so a recorded answer can be asserted against
/// without a network: what a live bugzilla returns is not something a test can
/// pin, but the shape is.
fn parse_open_bugs(source: &Bugzilla, body: &str) -> Result<Vec<Issue>> {
let list: BugList =
serde_json::from_str(body).map_err(|error| Error::BugzillaNotABugList { source: error })?;
Ok(list
.bugs
.into_iter()
.map(|bug| {
// Read the shared fields before `summary` moves out of the bug.
let component = place(&bug);
let url = source.issue_url(bug.id);
Issue {
id: bug.id.to_string(),
title: bug.summary,
component,
url,
}
})
.collect())
}
/// Where a bug lives, as far as the API says: `product/component` when it
/// names both, whichever one it names otherwise.
fn place(bug: &Bug) -> Option<String> {
match (&bug.product, &bug.component) {
(Some(product), Some(component)) => Some(format!("{product}/{component}")),
(Some(only), None) | (None, Some(only)) => Some(only.clone()),
(None, None) => None,
}
}
/// Whether an issue answers to a `--filter` word.
///
/// The title or the component containing it is a match, case-insensitively:
/// the words someone remembers from a bug are rarely the ones with the right
/// capitalisation.
fn matches(issue: &Issue, filter: &str) -> bool {
let needle = filter.to_lowercase();
issue.title.to_lowercase().contains(&needle)
|| issue
.component
.as_ref()
.is_some_and(|component| component.to_lowercase().contains(&needle))
}
/// Fetch the open issues and pick one.
///
/// TODO: apply `args.filter` to the title or component before picking, and
/// print JSON when `args.json` is set.
pub async fn run(config: &Config, args: QuestArgs) -> Result<Issue> {
let _ = (config, args);
todo!("pick one open bug at random")
/// The config is deliberately not consulted: a quest is a question about
/// bugzilla, not about a kernel tree, so this verb works before anything has
/// been cloned.
pub async fn run(_config: &Config, args: QuestArgs) -> Result<Issue> {
let source = Bugzilla::default();
let mut issues = source.fetch_open().await?;
if let Some(filter) = &args.filter {
issues.retain(|issue| matches(issue, filter));
}
let picked = pick_random(&issues, seed()).ok_or_else(|| match &args.filter {
Some(filter) => Error::NoQuestCandidates {
filter: filter.clone(),
},
None => Error::BugzillaEmpty,
})?;
Ok(picked.clone())
}
/// A seed that changes between runs, for a pick nobody can rerun by accident.
///
/// The low bits of the clock are the ones that move, and those are the ones a
/// caller of [`pick_random`] folds through the xorshift.
fn seed() -> u64 {
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map_or(0, |elapsed| elapsed.as_nanos() as u64)
}
/// Choose one issue at random. `seed` keeps the pick reproducible in tests.
pub fn pick_random(issues: &[Issue], seed: u64) -> Option<&Issue> {
todo!(
"index into {} candidate issues with seed {seed}",
issues.len()
)
if issues.is_empty() {
return None;
}
let index = (xorshift(seed) % issues.len() as u64) as usize;
issues.get(index)
}
/// One step of xorshift64, so a clock seed becomes a spread-out index without
/// a `rand` dependency.
fn xorshift(mut state: u64) -> u64 {
state ^= state << 13;
state ^= state >> 7;
state ^= state << 17;
state
}
#[cfg(test)]
mod tests {
use super::*;
/// A `/rest/bug` answer, recorded from the live API during planning and
/// trimmed to three bugs: one with both place fields, one with only a
/// component, and one with only a product.
const RECORDED: &str = r#"{
"bugs": [
{
"id": 219123,
"summary": "usb: xhci: device does not enumerate after resume",
"product": "Drivers",
"component": "USB"
},
{
"id": 218001,
"summary": "amdgpu: ring timeout on resume",
"component": "Video(AMDGPU/R600)"
},
{
"id": 215000,
"summary": "ppc64: build failure with gcc 15",
"product": "Platform Specific/Hardware"
}
]
}"#;
fn issues() -> Vec<Issue> {
parse_open_bugs(&Bugzilla::default(), RECORDED).expect("the recorded answer parses")
}
#[test]
fn a_recorded_answer_becomes_issues() {
let issues = issues();
assert_eq!(issues.len(), 3);
assert_eq!(issues[0].id, "219123");
assert_eq!(
issues[0].title,
"usb: xhci: device does not enumerate after resume"
);
assert_eq!(
issues[0].url,
"https://bugzilla.kernel.org/show_bug.cgi?id=219123"
);
}
#[test]
fn a_bug_keeps_whichever_place_fields_it_has() {
let issues = issues();
assert_eq!(issues[0].component.as_deref(), Some("Drivers/USB"));
assert_eq!(
issues[1].component.as_deref(),
Some("Video(AMDGPU/R600)"),
"a component with no product is still worth showing"
);
assert_eq!(
issues[2].component.as_deref(),
Some("Platform Specific/Hardware")
);
}
#[test]
fn an_answer_that_is_not_a_bug_list_is_a_named_error() {
let error = parse_open_bugs(&Bugzilla::default(), "<html>nope</html>")
.expect_err("html is not JSON");
assert!(error.to_string().contains("is not a bug list"), "{error}");
}
#[test]
fn an_answer_with_no_bugs_parses_as_none() {
assert_eq!(
parse_open_bugs(&Bugzilla::default(), r#"{"bugs": []}"#)
.expect("an empty list")
.len(),
0
);
}
#[test]
fn a_filter_matches_the_title_or_the_component_whatever_the_case() {
let issues = issues();
assert!(matches(&issues[0], "xhci"));
assert!(matches(&issues[0], "XHCI"));
assert!(matches(&issues[1], "amdgpu"));
assert!(matches(&issues[2], "platform specific"));
assert!(!matches(&issues[0], "amdgpu"));
}
#[test]
fn the_same_seed_picks_the_same_issue() {
let issues = issues();
for seed in [0, 1, 7, 42, 12345, u64::MAX] {
let first = pick_random(&issues, seed).expect("a pick");
let again = pick_random(&issues, seed).expect("the same pick");
assert_eq!(first.id, again.id, "seed {seed} was not stable");
}
}
#[test]
fn there_is_nothing_to_pick_from_an_empty_list() {
assert!(pick_random(&[], 1).is_none());
}
#[test]
fn the_seed_spreads_across_the_list() {
// Not a distribution assertion: only that consecutive seeds do not all
// land on the same issue, which is what folding nothing would do, and that
// every pick is one of the candidates.
let issues = issues();
let picked: std::collections::HashSet<&str> = (0..64)
.filter_map(|seed| pick_random(&issues, seed).map(|issue| issue.id.as_str()))
.collect();
assert!(picked.len() > 1, "every seed picked the same issue");
assert!(
picked
.iter()
.all(|id| issues.iter().any(|issue| issue.id == *id))
);
}
#[test]
fn the_json_form_is_json_and_holds_every_field() {
let rendered = issues()[0].render(true);
let parsed: serde_json::Value =
serde_json::from_str(&rendered).expect("the JSON form parses");
assert_eq!(parsed["id"], "219123");
assert_eq!(parsed["component"], "Drivers/USB");
assert_eq!(
parsed["url"],
"https://bugzilla.kernel.org/show_bug.cgi?id=219123"
);
assert!(parsed["title"].as_str().is_some_and(|t| t.contains("xhci")));
}
#[test]
fn the_readable_form_names_the_component_and_the_url() {
let rendered = issues()[1].render(false);
assert!(
rendered.contains("component: Video(AMDGPU/R600)"),
"{rendered}"
);
assert!(rendered.contains("show_bug.cgi?id=218001"), "{rendered}");
}
}
+40 -5
View File
@@ -6,6 +6,7 @@
mod cli;
mod config;
mod doctor;
mod error;
mod git;
mod kernel;
@@ -40,14 +41,27 @@ async fn run(cli: Cli) -> Result<ExitCode> {
match cli.command {
Command::Kernel { command } => match command {
KernelCommand::Quest(args) => {
let json = args.json;
let issue = kernel::quest::run(&config, args).await?;
println!("{} {}", issue.id, issue.title);
println!("{}", issue.url);
println!("{}", issue.render(json));
Ok(ExitCode::SUCCESS)
}
KernelCommand::Test(args) => kernel::qemu::run(&config, args)
.await
.map(|()| ExitCode::SUCCESS),
KernelCommand::Test(args) => {
let outcome = kernel::qemu::run(&config, args).await?;
println!("image: {}", outcome.image.display());
if outcome.succeeded() {
println!("qemu exited cleanly");
Ok(ExitCode::SUCCESS)
} else {
// A boot that failed is not a crash of spectral, so this
// reads as a result rather than as an error.
println!(
"qemu exited with status {}",
crate::error::exit_code(&outcome.status)
);
Ok(ExitCode::FAILURE)
}
}
},
Command::Patch { command } => match command {
@@ -89,6 +103,27 @@ async fn run(cli: Cli) -> Result<ExitCode> {
Ok(ExitCode::SUCCESS)
}
},
Command::Doctor => {
let report = doctor::run(&config);
print!("{}", report.render());
Ok(if report.ok() {
ExitCode::SUCCESS
} else {
ExitCode::FAILURE
})
}
Command::Init(args) => {
let init = config::init(&args)?;
if init.wrote {
println!("wrote {}", init.config_file.display());
} else {
println!("{} already says this", init.config_file.display());
}
println!("tree: {}", init.kernel_tree.display());
Ok(ExitCode::SUCCESS)
}
}
}
+32 -16
View File
@@ -26,29 +26,42 @@ pub struct Recipients {
pub touched: Vec<PathBuf>,
}
/// Look up recipients for a patch.
/// The arguments one lookup runs with.
///
/// One invocation over the patch. `--no-tree` because the script's own tree
/// check wants a dozen marker files a working tree need not have, and
/// `--pattern-depth=0` so a file matches every `F:` entry covering it rather
/// than only the closest one.
pub fn lookup(kernel_tree: &Path, patch: &Path) -> Result<Recipients> {
let patch_arg = patch.to_string_lossy().into_owned();
let output = run(
kernel_tree,
&[
/// `--no-tree` because the script's own tree check wants a dozen marker files a
/// working tree need not have, and `--pattern-depth=0` so a file matches every
/// `F:` entry covering it rather than only the closest one.
const LOOKUP_ARGS: [&str; 5] = [
"--no-tree",
"--git",
"--roles",
"--no-rolestats",
"--pattern-depth=0",
&patch_arg,
],
)?;
];
/// Look up recipients for a patch, or for every patch in a set.
///
/// One invocation over all the files: a series has one recipient list, and
/// asking per patch would only repeat the same answer.
pub fn lookup(kernel_tree: &Path, patches: &[PathBuf]) -> Result<Recipients> {
let mut args: Vec<String> = LOOKUP_ARGS.iter().map(|arg| (*arg).to_owned()).collect();
for patch in patches {
args.push(patch.to_string_lossy().into_owned());
}
let borrowed: Vec<&str> = args.iter().map(String::as_str).collect();
let output = run(kernel_tree, &borrowed)?;
let entries: Vec<Entry> = output.lines().filter_map(parse_entry).collect();
let mut touched: Vec<PathBuf> = Vec::new();
for patch in patches {
for file in touched_files(patch)? {
if !touched.contains(&file) {
touched.push(file);
}
}
}
Ok(split(&entries, touched_files(patch)?))
Ok(split(&entries, touched))
}
/// Add extra addresses to the CC list, skipping ones already there.
@@ -164,9 +177,12 @@ fn parse_entry(line: &str) -> Option<Entry> {
/// Paths git felt the need to quote are skipped rather than mangled. They are
/// for reporting only; the lookup itself reads the patch.
fn touched_files(patch: &Path) -> Result<Vec<PathBuf>> {
let patch = std::fs::read(patch)?;
let patch_bytes = std::fs::read(patch).map_err(|source| Error::PatchUnreadable {
path: patch.to_path_buf(),
source,
})?;
Ok(String::from_utf8_lossy(&patch)
Ok(String::from_utf8_lossy(&patch_bytes)
.lines()
.filter_map(|line| line.strip_prefix("diff --git "))
.filter_map(|rest| rest.rsplit_once(" b/"))
+223 -32
View File
@@ -15,6 +15,7 @@
pub mod checkpatch;
pub mod maintainers;
pub mod series;
use std::path::{Path, PathBuf};
@@ -62,17 +63,20 @@ pub fn commit(config: &Config, args: CommitArgs) -> Result<()> {
/// Write the change against `--base` out as a patch file.
///
/// The name is normalised to end in `.patch` and the file lands in
/// [`Config::patch_dir`] unless `--out` says otherwise. Returns the path
/// written.
/// One file, one commit: a range holding more than one commit is refused and
/// pointed at `--range`, because a single file cannot carry a set.
pub fn create(config: &Config, args: CreateArgs) -> Result<PathBuf> {
if args.cover_letter.is_some() && args.range.is_none() {
return Err(Error::CoverLetterNeedsSeries);
}
if let Some(range) = args.range.clone() {
return create_series(config, &args, &range);
}
let name = args.name.clone().ok_or(Error::CreateNeedsName)?;
let tree = config.require_kernel_tree()?;
let git = Git::new(tree);
let base = git
.rev_parse(&args.base)
.map_err(|_| Error::UnknownRevision {
rev: args.base.clone(),
})?;
let base = resolve_revision(&git, &args.base)?;
// The patch is mail-formatted rather than a bare diff: `git send-email`
// refuses the bare one with "No subject line".
@@ -90,12 +94,95 @@ pub fn create(config: &Config, args: CreateArgs) -> Result<PathBuf> {
let dir = args.out.unwrap_or_else(|| config.patch_dir().to_path_buf());
std::fs::create_dir_all(&dir)?;
let path = dir.join(patch_name(&args.name));
let path = dir.join(patch_name(&name));
std::fs::write(&path, set)?;
Ok(path)
}
/// Write a whole set, with a cover letter, into a directory of its own.
///
/// The directory carries a sidecar saying what was generated and at which
/// revision, which is what lets `patch update` re-roll the set without the
/// range being retyped.
fn create_series(config: &Config, args: &CreateArgs, range: &str) -> Result<PathBuf> {
// Checked before anything is written: a blurb with no first line would
// otherwise leave generated files on disk with no sidecar beside them.
if let Some(blurb) = &args.cover_letter
&& blurb.trim().is_empty()
{
return Err(Error::CoverLetterBlurbEmpty);
}
let tree = config.require_kernel_tree()?;
let git = Git::new(tree);
let resolved = resolve_range(&git, range)?;
let (base, head) = resolved.split_once("..").map_or_else(
|| (resolved.clone(), resolved.clone()),
|(base, head)| (base.to_owned(), head.to_owned()),
);
let dir = series_dir(config, args);
std::fs::create_dir_all(&dir)?;
let files = series::generate(&git, &dir, &resolved, 1, args.cover_letter.as_deref())?;
series::Series {
base,
head,
version: 1,
files,
cover_letter: args.cover_letter.clone(),
}
.write(&dir)?;
Ok(dir)
}
/// The directory a series is written into.
///
/// `NAME` names a subdirectory of the patch directory, which keeps a set from
/// mixing with the single patches beside it. Without a name the patch
/// directory itself is the series directory.
fn series_dir(config: &Config, args: &CreateArgs) -> PathBuf {
match (&args.out, &args.name) {
(Some(out), Some(name)) => out.join(name),
(Some(out), None) => out.clone(),
(None, Some(name)) => config.patch_dir().join(name),
(None, None) => config.patch_dir().to_path_buf(),
}
}
/// Turn `<base>..<head>` into hashes.
///
/// Both ends are resolved before format-patch runs: a branch that moves
/// between resolving and generating would otherwise change which commits the
/// set holds.
fn resolve_range(git: &Git, range: &str) -> Result<String> {
let (base, head) = range
.split_once("..")
.ok_or_else(|| Error::RangeNotARange {
range: range.to_owned(),
})?;
let head = head.strip_prefix('.').unwrap_or(head);
let base = if base.is_empty() { "HEAD" } else { base };
let head = if head.is_empty() { "HEAD" } else { head };
let base_hash = resolve_revision(git, base)?;
let head_hash = resolve_revision(git, head)?;
Ok(format!("{base_hash}..{head_hash}"))
}
/// One end of a range, or the named error saying which revision was missing.
///
/// A tree that is not a git repository is reported as itself rather than as a
/// missing revision: the two failures want different fixes.
fn resolve_revision(git: &Git, rev: &str) -> Result<String> {
git.rev_parse_optional(rev)?
.ok_or_else(|| Error::UnknownRevision {
rev: rev.to_owned(),
})
}
/// What a `submit` run decided, so `main` can print it.
#[derive(Debug, Clone)]
pub struct Submission {
@@ -108,7 +195,7 @@ pub struct Submission {
pub output: String,
}
/// Send a patch to whoever `get_maintainer.pl` names.
/// Send a patch, or a whole series, to whoever `get_maintainer.pl` names.
///
/// With `--dry-run` nothing leaves the machine: send-email prints the plan and
/// nothing else happens. send-email's own refusals, a cover letter still
@@ -116,10 +203,13 @@ pub struct Submission {
/// swallowed, because the exit code is what a script around this reads.
pub fn submit(config: &Config, args: SubmitArgs) -> Result<Submission> {
let tree = config.require_kernel_tree()?;
let patch = resolve_patch(config, &args.patch)?;
let patches = resolve_patches(config, &args.patches)?;
let mut recipients = maintainers::lookup(tree, &patch)?;
maintainers::add_cc(&mut recipients, &args.cc);
let mut recipients = maintainers::lookup(tree, &patches)?;
// `--cc` on the command line, then whatever the config file always copies.
let mut extras: Vec<String> = args.cc.clone();
extras.extend(config.always_cc().iter().cloned());
maintainers::add_cc(&mut recipients, &extras);
let mut argv: Vec<String> = vec!["send-email".to_owned()];
if args.dry_run {
@@ -128,6 +218,10 @@ pub fn submit(config: &Config, args: SubmitArgs) -> Result<Submission> {
// an answer to send-email's confirmation question.
argv.push("--confirm=never".to_owned());
}
if let Some(from) = config.send_email_from() {
argv.push("--from".to_owned());
argv.push(from);
}
for address in &recipients.to {
argv.push("--to".to_owned());
argv.push(address.clone());
@@ -140,7 +234,11 @@ pub fn submit(config: &Config, args: SubmitArgs) -> Result<Submission> {
argv.push("--in-reply-to".to_owned());
argv.push(message_id.clone());
}
// A set goes out in one invocation, which is what threads the patches
// together and what send-email already knows how to do.
for patch in &patches {
argv.push(patch.to_string_lossy().into_owned());
}
let borrowed: Vec<&str> = argv.iter().map(String::as_str).collect();
let git = Git::new(tree);
@@ -153,33 +251,93 @@ pub fn submit(config: &Config, args: SubmitArgs) -> Result<Submission> {
Ok(Submission {
recipients,
command: format!("git -C {} {}", tree.display(), shell_join(&argv)),
command: format!(
"git -C {} {}",
shell_quote(&tree.display().to_string()),
shell_join(&argv)
),
output,
})
}
/// A command line a reader can paste, with the awkward arguments quoted.
/// The patch files to send, in the order they will be sent.
///
/// A directory stands for a series: its sidecar decides the order when there
/// is one, and otherwise the patch names sorted, which is the order
/// format-patch numbered them in.
fn resolve_patches(config: &Config, names: &[PathBuf]) -> Result<Vec<PathBuf>> {
let mut files: Vec<PathBuf> = Vec::new();
for name in names {
let resolved = resolve_patch(config, name)?;
if !resolved.is_dir() {
files.push(resolved);
continue;
}
let inside = match series::Series::read(&resolved) {
Ok(series) => series.paths(&resolved),
Err(Error::SeriesManifestMissing { .. }) => series::patches_in(&resolved)?
.into_iter()
.map(|name| resolved.join(name))
.collect(),
Err(other) => return Err(other),
};
if inside.is_empty() {
return Err(Error::NoPatchesInDirectory { dir: resolved });
}
files.extend(inside);
}
Ok(files)
}
/// A command line a reader can paste.
///
/// Quoting an argument only when it holds a space is not enough to make that
/// true: `--in-reply-to <id@host>` is a redirection in bash, and an address
/// with an apostrophe swallows the opening quote and arrives mangled. Anything
/// outside a conservative safe set is single-quoted, and an apostrophe inside is
/// escaped the way a shell reads it.
fn shell_join(argv: &[String]) -> String {
argv.iter()
.map(|arg| {
if arg.contains(' ') {
format!("'{arg}'")
} else {
arg.clone()
}
})
.map(|arg| shell_quote(arg))
.collect::<Vec<_>>()
.join(" ")
}
/// Re-roll a patch as `v<N>`.
/// One argument, quoted if a shell would otherwise read something into it.
fn shell_quote(arg: &str) -> String {
let plain = !arg.is_empty()
&& arg.bytes().all(|byte| {
byte.is_ascii_alphanumeric()
|| matches!(
byte,
b'/' | b'.' | b'-' | b'_' | b'+' | b':' | b'=' | b'@' | b','
)
});
if plain {
arg.to_owned()
} else {
format!("'{}'", arg.replace('\'', r"'\''"))
}
}
/// Re-roll a patch as `v<N>`, or a whole series when handed a directory.
///
/// `000-kernel-patch.patch` becomes `v2-000-kernel-patch.patch`; an existing
/// `vN-` prefix is replaced rather than stacked, so re-running it at the same
/// revision is a no-op. `args.revision` overrides the inferred next number.
/// Returns the path of the renamed file.
/// A single patch is renamed: `000-kernel-patch.patch` becomes
/// `v2-000-kernel-patch.patch`, and an existing `vN-` prefix is replaced
/// rather than stacked, so re-running it at the same revision is a no-op.
/// A directory is regenerated through `git format-patch --reroll-count`, so
/// every file in it moves to the same revision. `args.revision` overrides the
/// inferred next number. Returns the path of what moved.
pub fn update(config: &Config, args: UpdateArgs) -> Result<PathBuf> {
let patch = resolve_patch(config, &args.patch)?;
if patch.is_dir() {
return update_series(config, &patch, args.revision);
}
let revision = match args.revision {
Some(revision) => revision,
None => current_revision(&patch) + 1,
@@ -198,6 +356,37 @@ pub fn update(config: &Config, args: UpdateArgs) -> Result<PathBuf> {
Ok(renamed)
}
/// Re-roll every file in a series directory to `revision`.
///
/// The range comes out of the sidecar rather than being rebuilt as `base..HEAD`:
/// a commit that landed on the branch after `create` is a different set, and
/// picking it up silently would mail commits nobody asked to send. The
/// superseded revision is removed only once the new one is entirely on disk, so
/// a re-roll that fails halfway leaves the previous revision usable.
fn update_series(config: &Config, dir: &Path, revision: Option<u32>) -> Result<PathBuf> {
let series = series::Series::read(dir)?;
let version = revision.unwrap_or(series.version + 1);
if version == series.version {
return Ok(dir.to_path_buf());
}
let tree = config.require_kernel_tree()?;
let git = Git::new(tree);
let range = format!("{}..{}", series.base, series.head);
let files = series::generate(&git, dir, &range, version, series.cover_letter.as_deref())?;
series::remove_superseded(dir, &series.files, &files)?;
series::Series {
base: series.base,
head: series.head,
version,
files,
cover_letter: series.cover_letter,
}
.write(dir)?;
Ok(dir.to_path_buf())
}
/// The path `patch` should have once it is revision `revision`.
///
/// Kept separate from [`update`] because it is the rule worth having a test
@@ -229,11 +418,13 @@ fn strip_revision(name: &str) -> &str {
/// Find the patch the user named.
///
/// A path that is there is used as given. Anything else is looked for in the
/// patch directory, which is what the README's examples assume: `patch submit
/// 000-xhci-port-rearm.patch` from anywhere, without the path in front.
/// A path that is there is used as given, a directory included: a series is a
/// directory, and `submit` and `update` both take one. Anything else is looked
/// for in the patch directory, which is what the README's examples assume:
/// `patch submit 000-xhci-port-rearm.patch` from anywhere, without the path in
/// front.
fn resolve_patch(config: &Config, name: &Path) -> Result<PathBuf> {
if name.is_file() {
if name.is_file() || name.is_dir() {
return Ok(name.to_path_buf());
}
@@ -242,7 +433,7 @@ fn resolve_patch(config: &Config, name: &Path) -> Result<PathBuf> {
|name| name.to_string_lossy().into_owned(),
);
let in_patch_dir = config.patch_dir().join(&file_name);
if in_patch_dir.is_file() {
if in_patch_dir.is_file() || in_patch_dir.is_dir() {
return Ok(in_patch_dir);
}
+222
View File
@@ -0,0 +1,222 @@
//! Patch sets: what a series directory is, and the `git format-patch` calls
//! that write one.
//!
//! The naming is not spectral's: `git format-patch` already numbers the files,
//! prefixes a re-roll with `vN`, and writes the cover letter. What is spectral's
//! is the sidecar that lets a re-roll know what it is re-rolling, and the
//! refusal to leave two revisions in one directory.
use std::path::{Path, PathBuf};
use serde::{Deserialize, Serialize};
use crate::error::{Error, Result};
use crate::git::Git;
/// The file a series directory carries.
pub const MANIFEST: &str = "series.toml";
/// The placeholder git leaves in a cover letter's subject line.
const SUBJECT_TEMPLATE: &str = "*** SUBJECT HERE ***";
/// The placeholder git leaves where the blurb belongs.
const BLURB_TEMPLATE: &str = "*** BLURB HERE ***";
/// What a series directory holds.
///
/// Minimal on purpose: the range, the revision, the files, and the cover letter
/// blurb. Where sent patches are tracked afterwards is a later design question
/// and is deliberately not answered here.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct Series {
/// Commit the set was generated against, resolved to a hash so a branch
/// moving under a re-roll cannot change what the range means.
pub base: String,
/// The other end of that range, also a hash.
///
/// Without it a re-roll would regenerate `base..HEAD`, which quietly picks
/// up every commit that landed on the branch since `create` and mails it
/// with the set.
pub head: String,
/// The revision every file in the directory is at.
pub version: u32,
/// The files, in the order they are sent: cover letter first.
pub files: Vec<String>,
/// The cover letter blurb, kept so a re-roll does not throw it away and
/// regenerate the template that `git send-email` refuses.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cover_letter: Option<String>,
}
impl Series {
/// Read the sidecar out of a series directory.
pub fn read(dir: &Path) -> Result<Self> {
let path = dir.join(MANIFEST);
let text = match std::fs::read_to_string(&path) {
Ok(text) => text,
// Only "there is no manifest here" means "this is not a series".
// Anything else is a real read failure and is reported as one.
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
return Err(Error::SeriesManifestMissing { path });
}
Err(error) => return Err(Error::Io(error)),
};
toml::from_str(&text).map_err(|source| Error::SeriesManifestUnreadable { path, source })
}
/// Write the sidecar into a series directory.
pub fn write(&self, dir: &Path) -> Result<()> {
let text =
toml::to_string(self).map_err(|source| Error::SeriesManifestUnwritable { source })?;
std::fs::write(dir.join(MANIFEST), text)?;
Ok(())
}
/// The patches this series holds, in the order they are sent.
#[must_use]
pub fn paths(&self, dir: &Path) -> Vec<PathBuf> {
self.files.iter().map(|name| dir.join(name)).collect()
}
}
/// Generate the set for `range` into `dir`, and return the file names written.
///
/// `version` above 1 goes through `--reroll-count`, which is what puts the same
/// `vN-` prefix on every file including the cover letter. The names come from
/// what format-patch says it wrote rather than from listing the directory: on a
/// re-roll the directory still holds the revision being replaced, and a listing
/// would read those back as part of the new set.
pub fn generate(
git: &Git,
dir: &Path,
range: &str,
version: u32,
cover_letter: Option<&str>,
) -> Result<Vec<String>> {
let mut args = vec!["format-patch".to_owned(), "--cover-letter".to_owned()];
if version > 1 {
args.push(format!("--reroll-count={version}"));
}
args.push("-o".to_owned());
args.push(dir.display().to_string());
args.push(range.to_owned());
let borrowed: Vec<&str> = args.iter().map(String::as_str).collect();
let written = git.run(&borrowed)?;
let files: Vec<String> = written
.lines()
.map(str::trim)
.filter(|line| !line.is_empty())
.map(|line| {
Path::new(line).file_name().map_or_else(
|| line.to_owned(),
|name| name.to_string_lossy().into_owned(),
)
})
.collect();
if files.is_empty() {
return Err(Error::RangeHoldsNoCommits {
range: range.to_owned(),
});
}
if let Some(blurb) = cover_letter {
fill_cover_letter(dir, &files, blurb)?;
}
Ok(files)
}
/// Every patch in `dir`, cover letter first.
///
/// format-patch numbers the files, so their names sort into the order they are
/// meant to be sent in.
pub fn patches_in(dir: &Path) -> Result<Vec<String>> {
let entries = std::fs::read_dir(dir)?;
let mut names: Vec<String> = Vec::new();
for entry in entries {
let entry = entry?;
let name = entry.file_name().to_string_lossy().into_owned();
if name.ends_with(".patch") && entry.path().is_file() {
names.push(name);
}
}
names.sort();
Ok(names)
}
/// Put the blurb into a freshly generated cover letter.
///
/// git leaves `*** SUBJECT HERE ***` and `*** BLURB HERE ***` behind, and
/// `git send-email` refuses to send a patch carrying either, so a series
/// created without this stays unsendable on purpose: the refusal is louder than
/// a cover letter nobody wrote.
fn fill_cover_letter(dir: &Path, files: &[String], blurb: &str) -> Result<()> {
let Some(cover) = files.iter().find(|name| name.contains("cover-letter")) else {
return Err(Error::CoverLetterMissing {
dir: dir.to_path_buf(),
});
};
let path = dir.join(cover);
let original = std::fs::read_to_string(&path)?;
let subject = blurb.lines().next().unwrap_or_default().trim().to_owned();
if subject.is_empty() {
return Err(Error::CoverLetterBlurbEmpty);
}
let mut filled = String::with_capacity(original.len() + blurb.len());
for line in original.lines() {
match line
.strip_prefix("Subject: ")
.and_then(|subject| subject.rsplit_once(SUBJECT_TEMPLATE))
{
Some((head, _)) => {
filled.push_str("Subject: ");
filled.push_str(head);
filled.push_str(&subject);
}
None => filled.push_str(line),
}
filled.push('\n');
}
let filled = filled.replace(BLURB_TEMPLATE, blurb.trim_end());
std::fs::write(&path, filled)?;
Ok(())
}
/// Delete the files of the revision this one replaces.
///
/// Called only once every new file is on disk, so a failed re-roll cannot cost
/// the previous revision. A file that is already gone is not a failure, because
/// the directory is then in the state the re-roll wanted; a file that will not
/// go is named rather than swallowed, since two revisions in one directory is
/// what `submit` would send.
pub fn remove_superseded(dir: &Path, old: &[String], new: &[String]) -> Result<()> {
let mut stuck: Vec<String> = Vec::new();
for name in old {
if new.contains(name) || name == MANIFEST {
continue;
}
if let Err(error) = std::fs::remove_file(dir.join(name))
&& error.kind() != std::io::ErrorKind::NotFound
{
stuck.push(format!("{name} ({error})"));
}
}
if stuck.is_empty() {
Ok(())
} else {
Err(Error::StaleSeriesFiles {
names: stuck.join(", "),
})
}
}
+2 -13
View File
@@ -29,6 +29,8 @@ fn every_verb_has_help_of_its_own() {
&["patch", "create"],
&["patch", "submit"],
&["patch", "update"],
&["doctor"],
&["init"],
] {
let mut args = verb.to_vec();
args.push("--help");
@@ -67,16 +69,3 @@ fn a_leaf_missing_its_argument_exits_two() {
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("MESSAGE"), "{stderr}");
}
#[test]
fn a_verb_that_is_still_a_stub_panics_where_it_says_it_does() {
// `kernel quest` is P5 and `kernel test` is P6. Until they land, this is
// the proof that the dispatch is wired end to end: exit 101 with the
// panic's own location in it.
let fixture = Fixture::kernel_tree();
let output = fixture.cli(&["kernel", "quest"]);
assert_eq!(output.status.code(), Some(101));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("kernel/quest.rs"), "{stderr}");
}
+93
View File
@@ -161,6 +161,17 @@ impl Fixture {
std::fs::set_permissions(&path, permissions).expect("make the script executable");
}
/// Write an executable script into a directory of its own.
fn write_script_in(&self, dir: &Path, name: &str, body: &str) {
let path = dir.join(name);
std::fs::write(&path, body).expect("write the script");
let mut permissions = std::fs::metadata(&path)
.expect("stat the script")
.permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&path, permissions).expect("make the script executable");
}
pub fn read(&self, rel: &str) -> String {
std::fs::read_to_string(self.join(rel)).expect("read the file")
}
@@ -214,6 +225,40 @@ impl Fixture {
self.cli_env(args, &[])
}
/// Run the built binary without `$SPECTRAL_KERNEL`, so the config file and
/// the built-in default are the only things left to resolve a tree from.
///
/// `XDG_CONFIG_HOME` points into the fixture, which is where the test's
/// config file lives.
pub fn cli_without_the_tree_variable(&self, args: &[&str], env: &[(&str, &str)]) -> Output {
let mut command = Command::new(env!("CARGO_BIN_EXE_spectral"));
command
.args(args)
.current_dir(self.root.path())
.env_clear()
.env("PATH", std::env::var("PATH").unwrap_or_default())
.env("HOME", self.home.path())
.env("XDG_CONFIG_HOME", self.home.join("config"))
.env("GIT_CONFIG_GLOBAL", self.home.join("gitconfig"))
.env("GIT_CONFIG_SYSTEM", "/dev/null");
for (key, value) in env {
command.env(key, value);
}
command.output().expect("run spectral")
}
/// Write a config file into the fixture's own config directory.
pub fn write_config(&self, contents: &str) -> PathBuf {
let path = self.home.join("config/spectral/config.toml");
std::fs::create_dir_all(path.parent().expect("the config directory"))
.expect("create the config directory");
std::fs::write(&path, contents).expect("write the config file");
path
}
/// Run the built binary with extra environment on top of the fixture's.
pub fn cli_env(&self, args: &[&str], env: &[(&str, &str)]) -> Output {
let mut command = Command::new(env!("CARGO_BIN_EXE_spectral"));
@@ -246,4 +291,52 @@ impl Fixture {
command
}
/// A `PATH` holding symlinks to exactly these programs, found on the
/// machine's own path.
///
/// This is how a test takes a tool away: uninstalling qemu to prove the
/// missing-qemu path would be absurd, and pointing `PATH` somewhere narrow
/// asserts the same thing without touching the host.
pub fn restricted_path(&self, programs: &[&str]) -> PathBuf {
let bin = self.home.join("bin");
std::fs::create_dir_all(&bin).expect("create the restricted bin directory");
for program in programs {
let found = self.program(program);
std::os::unix::fs::symlink(found, bin.join(program)).expect("link the program");
}
bin
}
/// A `PATH` whose `git` is the machine's, except that `send-email` answers
/// with `answer` instead of running.
///
/// The harness controls `PATH` and git's configuration but not git's own
/// exec-path, so whether the host has git's send-email support installed is
/// otherwise inherited from the machine. This makes the answer the test's,
/// in both directions.
pub fn path_with_send_email(&self, answer: u32) -> PathBuf {
let bin = self.home.join("gitbin");
std::fs::create_dir_all(&bin).expect("create the git bin directory");
let real = self.program("git");
let wrapper = format!(
"#!/bin/sh\nif [ \"$1\" = send-email ]; then exit {answer}; fi\nexec {} \"$@\"\n",
real.display()
);
self.write_script_in(&bin, "git", &wrapper);
bin
}
/// The machine's own copy of a program, found on its `PATH`.
fn program(&self, name: &str) -> PathBuf {
std::env::var("PATH")
.unwrap_or_default()
.split(':')
.map(|dir| Path::new(dir).join(name))
.find(|candidate| candidate.is_file())
.unwrap_or_else(|| panic!("{name} is not on this machine's PATH"))
}
}
+373
View File
@@ -0,0 +1,373 @@
//! The config file and `spectral init`, driven through the built binary.
//!
//! Precedence is asserted at the command line rather than in a unit test,
//! because `std::env::set_var` is `unsafe` under edition 2024 and this crate
//! allows no `unsafe`: the process under test is the one that sets the
//! variables.
mod common;
use std::path::Path;
use common::Fixture;
fn stdout_of(output: &std::process::Output) -> String {
String::from_utf8_lossy(&output.stdout).into_owned()
}
fn stderr_of(output: &std::process::Output) -> String {
String::from_utf8_lossy(&output.stderr).into_owned()
}
/// A second kernel tree, so a test can tell two answers apart.
fn other_tree(fixture: &Fixture) -> std::path::PathBuf {
let tree = fixture.join("other-tree");
std::fs::create_dir_all(tree.join("scripts")).expect("create the other tree");
std::fs::write(tree.join("scripts/checkpatch.pl"), "#!/bin/sh\n").expect("write checkpatch");
std::fs::write(tree.join("scripts/get_maintainer.pl"), "#!/bin/sh\n")
.expect("write the script");
std::fs::write(tree.join("MAINTAINERS"), "OTHER DRIVER\n").expect("write MAINTAINERS");
tree
}
#[test]
fn a_config_file_supplies_the_tree_when_the_environment_does_not() {
let fixture = Fixture::kernel_tree();
fixture.write_config(&format!(
"kernel_tree = \"{}\"\npatch_dir = \"{}\"\n",
fixture.tree().display(),
fixture.home().join("patches").display()
));
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert!(
output.status.success(),
"{}{}",
stdout_of(&output),
stderr_of(&output)
);
let stdout = stdout_of(&output);
assert!(stdout.contains("ok kernel tree"), "{stdout}");
assert!(
stdout.contains("from the config file"),
"the report says the file answered: {stdout}"
);
assert!(
stdout.contains("ok config file"),
"the file is a check of its own: {stdout}"
);
}
#[test]
fn the_environment_beats_the_config_file() {
let fixture = Fixture::kernel_tree();
fixture.write_config(&format!(
"kernel_tree = \"{}\"\n",
other_tree(&fixture).display()
));
let output = fixture.cli_without_the_tree_variable(
&["doctor"],
&[(
"SPECTRAL_KERNEL",
fixture.tree().to_str().expect("the tree"),
)],
);
assert!(output.status.success(), "{}", stdout_of(&output));
let stdout = stdout_of(&output);
assert!(stdout.contains("from $SPECTRAL_KERNEL"), "{stdout}");
assert!(
stdout.contains(fixture.tree().to_str().expect("the tree")),
"{stdout}"
);
}
#[test]
fn the_file_beats_the_built_in_default() {
let fixture = Fixture::kernel_tree();
let other = other_tree(&fixture);
fixture.write_config(&format!("kernel_tree = \"{}\"\n", other.display()));
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert!(output.status.success(), "{}", stdout_of(&output));
let stdout = stdout_of(&output);
assert!(
stdout.contains(other.to_str().expect("the other tree")),
"the file's tree is the one doctor looked at: {stdout}"
);
assert!(
!stdout.contains(".spectral/linux"),
"the default was not used: {stdout}"
);
}
#[test]
fn with_no_file_and_no_variable_the_default_is_used() {
let fixture = Fixture::kernel_tree();
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert_eq!(output.status.code(), Some(1));
let stdout = stdout_of(&output);
assert!(
stdout.contains(".spectral/linux"),
"the default is $HOME/.spectral/linux: {stdout}"
);
assert!(stdout.contains("the built-in default"), "{stdout}");
}
#[test]
fn malformed_config_is_a_named_error_rather_than_a_panic() {
let fixture = Fixture::kernel_tree();
let path = fixture.write_config("kernel_tree = 42\n");
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert_eq!(
output.status.code(),
Some(1),
"a bad config is not a panic: {}{}",
stdout_of(&output),
stderr_of(&output)
);
let stderr = stderr_of(&output);
assert!(stderr.contains("is not a readable config file"), "{stderr}");
assert!(stderr.contains(&path.display().to_string()), "{stderr}");
}
#[test]
fn an_unknown_key_is_refused_rather_than_ignored() {
let fixture = Fixture::kernel_tree();
fixture.write_config(&format!(
"kernel_tree = \"{}\"\ntree = \"a typo\"\n",
fixture.tree().display()
));
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert_eq!(output.status.code(), Some(1));
assert!(
stderr_of(&output).contains("is not a readable config file"),
"{}",
stderr_of(&output)
);
}
#[test]
fn a_relative_path_in_the_config_is_refused() {
let fixture = Fixture::kernel_tree();
fixture.write_config("kernel_tree = \"linux\"\n");
let output = fixture.cli_without_the_tree_variable(&["doctor"], &[]);
assert_eq!(output.status.code(), Some(1));
let stderr = stderr_of(&output);
assert!(stderr.contains("not an absolute path"), "{stderr}");
}
#[test]
fn init_writes_the_config_and_a_second_run_changes_nothing() {
let fixture = Fixture::kernel_tree();
let first = fixture.cli_without_the_tree_variable(
&["init", "--tree", fixture.tree().to_str().expect("the tree")],
&[],
);
assert!(
first.status.success(),
"{}{}",
stdout_of(&first),
stderr_of(&first)
);
assert!(stdout_of(&first).contains("wrote"), "{}", stdout_of(&first));
let config = fixture.home().join("config/spectral/config.toml");
let written = std::fs::read(&config).expect("the config file");
assert!(
String::from_utf8_lossy(&written).contains(fixture.tree().to_str().expect("the tree")),
"{}",
String::from_utf8_lossy(&written)
);
let second = fixture.cli_without_the_tree_variable(
&["init", "--tree", fixture.tree().to_str().expect("the tree")],
&[],
);
assert!(second.status.success(), "{}", stderr_of(&second));
assert!(
stdout_of(&second).contains("already says this"),
"{}",
stdout_of(&second)
);
assert_eq!(
std::fs::read(&config).expect("the config file"),
written,
"the second run rewrote the file"
);
}
#[test]
fn init_refuses_to_clobber_a_config_that_says_something_else() {
let fixture = Fixture::kernel_tree();
let other = other_tree(&fixture);
fixture.write_config(&format!("kernel_tree = \"{}\"\n", other.display()));
let refused = fixture.cli_without_the_tree_variable(
&["init", "--tree", fixture.tree().to_str().expect("the tree")],
&[],
);
assert_eq!(refused.status.code(), Some(1));
let stderr = stderr_of(&refused);
assert!(stderr.contains("already points somewhere else"), "{stderr}");
assert!(stderr.contains("--force"), "{stderr}");
let forced = fixture.cli_without_the_tree_variable(
&[
"init",
"--tree",
fixture.tree().to_str().expect("the tree"),
"--force",
],
&[],
);
assert!(
forced.status.success(),
"{}{}",
stdout_of(&forced),
stderr_of(&forced)
);
let config = std::fs::read_to_string(fixture.home().join("config/spectral/config.toml"))
.expect("the config file");
assert!(
config.contains(fixture.tree().to_str().expect("the tree")),
"{config}"
);
}
#[test]
fn init_verifies_the_tree_it_is_pointed_at() {
let fixture = Fixture::kernel_tree();
let not_a_tree = fixture.join("not-a-tree");
std::fs::create_dir_all(&not_a_tree).expect("create the directory");
let output = fixture.cli_without_the_tree_variable(
&[
"init",
"--tree",
not_a_tree.to_str().expect("the directory"),
],
&[],
);
assert_eq!(output.status.code(), Some(1));
assert!(
stderr_of(&output).contains("not a kernel source tree"),
"{}",
stderr_of(&output)
);
assert!(
!Path::new(&fixture.home().join("config/spectral/config.toml")).exists(),
"a tree that did not verify must not be written into the config"
);
}
#[test]
fn init_refuses_before_it_clones_anything() {
// The refusal is about the config file, so it has to happen before a
// multi-minute kernel clone rather than after one has already landed.
let fixture = Fixture::kernel_tree();
let other = other_tree(&fixture);
fixture.write_config(&format!("kernel_tree = \"{}\"\n", other.display()));
let into = fixture.home().join("should-not-exist");
let output = fixture.cli_without_the_tree_variable(
&[
"init",
"--clone",
fixture.tree().to_str().expect("the tree"),
"--tree",
into.to_str().expect("the destination"),
],
&[],
);
assert_eq!(output.status.code(), Some(1));
assert!(
stderr_of(&output).contains("already points somewhere else"),
"{}",
stderr_of(&output)
);
assert!(
!into.exists(),
"the clone ran before the refusal was printed"
);
}
#[test]
fn init_clones_a_tree_it_was_given_a_url_for() {
// A local path is a url git can clone, so this exercises the clone branch
// without a network.
let fixture = Fixture::kernel_tree();
let into = fixture.home().join("cloned");
let output = fixture.cli_without_the_tree_variable(
&[
"init",
"--clone",
fixture.tree().to_str().expect("the tree"),
"--tree",
into.to_str().expect("the destination"),
],
&[],
);
assert!(
output.status.success(),
"{}{}",
stdout_of(&output),
stderr_of(&output)
);
assert!(into.join("scripts/checkpatch.pl").is_file());
assert!(into.join("MAINTAINERS").is_file());
let config = std::fs::read_to_string(fixture.home().join("config/spectral/config.toml"))
.expect("the config file");
assert!(config.contains("cloned"), "{config}");
}
#[test]
fn the_config_file_carries_always_cc_and_an_identity_into_submit() {
let fixture = Fixture::kernel_tree();
fixture.work_on_a_branch();
fixture.write("drivers/foo/bar.c", common::EDITED);
fixture.commit_all("foo: return a + 1");
assert!(
fixture
.cli(&["patch", "create", "000-foo"])
.status
.success()
);
fixture.write_config(&format!(
"kernel_tree = \"{}\"\nalways_cc = [\"[email protected]\"]\n\n[send_email]\nfrom = \"Someone <[email protected]>\"\n",
fixture.tree().display()
));
let output = fixture
.cli_without_the_tree_variable(&["patch", "submit", "000-foo.patch", "--dry-run"], &[]);
assert!(
output.status.success(),
"{}{}",
stdout_of(&output),
stderr_of(&output)
);
let stdout = stdout_of(&output);
assert!(stdout.contains("Cc: [email protected]"), "{stdout}");
assert!(
stdout.contains("--from 'Someone <[email protected]>'"),
"{stdout}"
);
assert!(stdout.contains("Dry-OK"), "{stdout}");
}
+216
View File
@@ -0,0 +1,216 @@
//! `spectral doctor`, driven through the built binary.
//!
//! The fixture controls `PATH` and git's configuration, so a missing qemu, a
//! missing sender identity and a tree that is not a kernel tree are produced on
//! purpose rather than inherited from whichever machine this runs on. That
//! extends to git's own send-email support, which the harness cannot control
//! through git's configuration: a `git` wrapper answers for it in both
//! directions, so a host without it still passes.
//!
//! Every test name here carries the verb, so the plan's own command
//! `cargo test --locked doctor` selects exactly this file's tests.
mod common;
use common::Fixture;
fn stdout_of(output: &std::process::Output) -> String {
String::from_utf8_lossy(&output.stdout).into_owned()
}
fn stderr_of(output: &std::process::Output) -> String {
String::from_utf8_lossy(&output.stderr).into_owned()
}
/// Run `doctor` with a `git` that has send-email support, whatever the host has.
fn doctor_with_send_email(fixture: &Fixture) -> std::process::Output {
let bin = fixture.path_with_send_email(0);
fixture.cli_env(
&["doctor"],
&[("PATH", bin.to_str().expect("the git bin directory"))],
)
}
#[test]
fn doctor_passes_on_a_complete_fixture_tree() {
let fixture = Fixture::kernel_tree();
let output = doctor_with_send_email(&fixture);
assert!(
output.status.success(),
"{}{}",
stdout_of(&output),
stderr_of(&output)
);
let stdout = stdout_of(&output);
assert!(!stdout.contains("fail "), "{stdout}");
assert!(stdout.contains("ok kernel tree"), "{stdout}");
assert!(
stdout.contains("from $SPECTRAL_KERNEL"),
"the report has to say which knob set the path: {stdout}"
);
assert!(stdout.contains("ok git send-email"), "{stdout}");
assert!(
stdout.contains("nothing failed") || stdout.contains("everything checked out"),
"{stdout}"
);
}
#[test]
fn doctor_names_the_first_hard_failure_when_the_tree_is_missing() {
let fixture = Fixture::bare();
let output = fixture.cli_env(&["doctor"], &[("SPECTRAL_KERNEL", "/nonexistent/linux")]);
assert_eq!(output.status.code(), Some(1));
let stdout = stdout_of(&output);
assert!(stdout.contains("fail kernel tree"), "{stdout}");
assert!(stdout.contains("first failure: kernel tree"), "{stdout}");
assert!(
stdout.contains("skip checkpatch.pl"),
"the checks that need a tree are not answers when there is none: {stdout}"
);
assert!(stdout.contains("-> "), "the fix hint is printed: {stdout}");
}
#[test]
fn doctor_names_a_directory_that_is_not_a_kernel_tree() {
let fixture = Fixture::bare();
let tree = fixture.join("not-a-tree");
std::fs::create_dir_all(&tree).expect("create the directory");
let output = fixture.cli_env(
&["doctor"],
&[("SPECTRAL_KERNEL", tree.to_str().expect("the tree path"))],
);
assert_eq!(output.status.code(), Some(1));
let stdout = stdout_of(&output);
assert!(stdout.contains("fail kernel tree shape"), "{stdout}");
assert!(stdout.contains("not a kernel source tree"), "{stdout}");
assert!(
stdout.contains("first failure: kernel tree shape"),
"{stdout}"
);
}
#[test]
fn doctor_warns_about_qemu_off_the_path_rather_than_failing() {
let fixture = Fixture::kernel_tree();
// A `PATH` holding the git wrapper and nothing else: qemu is gone, the
// rest is not.
let bin = fixture.path_with_send_email(0);
let output = fixture.cli_env(
&["doctor"],
&[("PATH", bin.to_str().expect("the git bin directory"))],
);
assert!(
output.status.success(),
"{}{}",
stdout_of(&output),
stderr_of(&output)
);
let stdout = stdout_of(&output);
assert!(stdout.contains("warn qemu"), "{stdout}");
assert!(
!stdout.contains("fail "),
"a missing qemu is not something the loop cannot work around: {stdout}"
);
}
#[test]
fn doctor_fails_on_no_sender_identity_and_says_how_to_fix_it() {
let fixture = Fixture::kernel_tree();
// Both places git would find one: the tree's own config and the fixture's
// global one.
fixture.git(&["config", "--unset", "user.email"]);
std::fs::write(
fixture.home().join("gitconfig"),
"[core]\n\trepositoryformatversion = 0\n",
)
.expect("write a global config with no identity");
let output = doctor_with_send_email(&fixture);
assert_eq!(output.status.code(), Some(1));
let stdout = stdout_of(&output);
assert!(stdout.contains("fail sender identity"), "{stdout}");
assert!(
stdout.contains("user.email"),
"the failure says how to fix it: {stdout}"
);
assert!(
stdout.contains("first failure: sender identity"),
"{stdout}"
);
}
#[test]
fn doctor_warns_when_git_has_no_send_email_support() {
// The direction the host cannot be trusted for: this machine has it, and a
// machine without it must fail the same way rather than pass by accident.
let fixture = Fixture::kernel_tree();
let bin = fixture.path_with_send_email(1);
let output = fixture.cli_env(
&["doctor"],
&[("PATH", bin.to_str().expect("the git bin directory"))],
);
assert_eq!(output.status.code(), Some(1));
let stdout = stdout_of(&output);
assert!(stdout.contains("fail git send-email"), "{stdout}");
assert!(stdout.contains("first failure: git send-email"), "{stdout}");
}
#[test]
fn doctor_warns_about_no_mail_route_because_dry_runs_are_what_matter() {
// The fixture's git config has no sendemail.* at all, which is the state
// this machine's own config was in when --dry-run was verified.
let fixture = Fixture::kernel_tree();
let output = doctor_with_send_email(&fixture);
assert!(output.status.success());
let stdout = stdout_of(&output);
assert!(stdout.contains("warn mail route"), "{stdout}");
assert!(
stdout.contains("--dry-run works"),
"the warning says what still works: {stdout}"
);
}
#[test]
fn doctor_reports_where_the_paths_came_from() {
let fixture = Fixture::kernel_tree();
let patches = fixture.home().join("patches");
let output = fixture.cli_env(
&["doctor"],
&[
(
"PATH",
fixture.path_with_send_email(0).to_str().expect("the bin"),
),
(
"SPECTRAL_PATCH_DIR",
patches.to_str().expect("the patch dir"),
),
],
);
let stdout = stdout_of(&output);
assert!(stdout.contains("from $SPECTRAL_PATCH_DIR"), "{stdout}");
assert!(
stdout.contains(patches.to_str().expect("the patch dir")),
"{stdout}"
);
assert!(
patches.is_dir(),
"doctor is the one check with a side effect: it makes the directory it was pointed at"
);
}
+44 -6
View File
@@ -152,6 +152,24 @@ fn a_bare_diff_is_refused_by_send_email() {
assert!(stderr.contains("No subject line"), "{stderr}");
}
#[test]
fn a_commit_with_nothing_staged_says_what_git_said() {
// git explains "nothing staged" on stdout, so an error path that only
// reads stderr prints a failure with nothing after it.
let fixture = Fixture::kernel_tree();
fixture.write("drivers/foo/bar.c", common::EDITED);
let output = fixture.cli(&["patch", "commit", "foo: nothing staged"]);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("failed (exit 1)"), "{stderr}");
assert!(
stderr.contains("no changes added to commit") || stderr.contains("not staged"),
"git's own explanation was dropped: {stderr}"
);
}
#[test]
fn create_names_a_base_that_does_not_resolve() {
let fixture = Fixture::kernel_tree();
@@ -180,8 +198,8 @@ fn create_refuses_a_range_that_holds_more_than_one_commit() {
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("holds 2 commits"), "{stderr}");
assert!(
stderr.contains("series support is not built yet"),
"{stderr}"
stderr.contains("--range"),
"the refusal has to say where a set belongs: {stderr}"
);
}
@@ -229,7 +247,7 @@ fn check_pipes_the_working_tree_diff_into_checkpatch_and_fails_on_errors() {
}
#[test]
fn warnings_alone_do_not_fail_the_command() {
fn checkpatch_warnings_alone_do_not_fail_the_command() {
let fixture = Fixture::kernel_tree();
fixture.write("drivers/foo/bar.c", common::EDITED);
@@ -246,7 +264,7 @@ fn warnings_alone_do_not_fail_the_command() {
}
#[test]
fn a_run_with_no_total_line_counts_as_clean() {
fn checkpatch_counts_a_run_with_no_total_line_as_clean() {
// What a clean run looks like under checkpatch's own --terse, where the
// summary line is suppressed entirely.
let fixture = Fixture::kernel_tree();
@@ -524,12 +542,32 @@ fn submit_threads_a_reroll_off_the_patch_it_replaces() {
assert!(output.status.success());
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("--in-reply-to <[email protected]>"),
"{stdout}"
stdout.contains("--in-reply-to '<[email protected]>'"),
"an unquoted <...> is a redirection when pasted: {stdout}"
);
assert!(stdout.contains("In-Reply-To: <[email protected]>"), "{stdout}");
}
#[test]
fn a_tree_that_is_not_a_repository_says_so_rather_than_blaming_a_revision() {
// `git rev-parse --verify --quiet master` fails both for a revision that is
// not there and for a directory that is not a repository. They want
// different fixes, so they get different messages.
let fixture = Fixture::bare();
fixture.write_script("scripts/checkpatch.pl", "#!/bin/sh\n");
fixture.write_script("scripts/get_maintainer.pl", "#!/bin/sh\n");
let output = fixture.cli(&["patch", "create", "000-foo"]);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("not a git repository"), "{stderr}");
assert!(
!stderr.contains("no revision"),
"a missing repository is not a missing revision: {stderr}"
);
}
#[test]
fn submit_refuses_a_template_cover_letter_instead_of_swallowing_it() {
let fixture = Fixture::kernel_tree();
+135
View File
@@ -0,0 +1,135 @@
//! `kernel test`, driven through the built binary against a fixture tree.
//!
//! Nothing here boots a kernel: qemu is taken off `PATH` deliberately, so the
//! assertions are about the paths around the boot rather than about a machine
//! that can run one. The real boot is the plan's P6 manual step.
mod common;
use common::Fixture;
/// A fixture tree with something at the path qemu would be handed.
fn fixture_with_image() -> Fixture {
let fixture = Fixture::kernel_tree();
fixture.write("arch/x86/boot/bzImage", "not really a kernel\n");
fixture.commit_all("fixture: add an image");
fixture
}
#[test]
fn a_missing_qemu_is_named_rather_than_panicked_on() {
let fixture = fixture_with_image();
// A `PATH` with nothing on it: the binary itself is found by its full
// path, and qemu cannot be.
let bare = fixture.restricted_path(&[]);
let output = fixture.cli_env(
&["kernel", "test", "--no-build"],
&[("PATH", bare.to_str().expect("the restricted path"))],
);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("could not run `qemu-system-x86_64`"),
"{stderr}"
);
assert!(
!String::from_utf8_lossy(&output.stdout).contains("exited cleanly"),
"a boot that never started was reported as a success"
);
}
#[test]
fn a_tree_with_no_image_is_named_rather_than_a_build_being_attempted() {
let fixture = Fixture::kernel_tree();
let output = fixture.cli(&["kernel", "test", "--no-build"]);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("no bootable image at"), "{stderr}");
assert!(stderr.contains("arch/x86/boot/bzImage"), "{stderr}");
}
#[test]
fn build_runs_make_in_the_tree_and_finds_the_image_it_wrote() {
// A Makefile that writes the image where a real build leaves it, and
// records that it ran. `sh` is the only tool it needs, so the restricted
// `PATH` can hold make and sh and still leave qemu out.
let fixture = Fixture::kernel_tree();
fixture.write("arch/x86/boot/.keep", "");
fixture.write(
"Makefile",
"all:\n\tprintf '' > arch/x86/boot/bzImage\n\tprintf 'built\\n' > .make-call\n",
);
fixture.commit_all("fixture: add a Makefile");
let bin = fixture.restricted_path(&["make", "sh"]);
let output = fixture.cli_env(
&["kernel", "test"],
&[("PATH", bin.to_str().expect("the restricted path"))],
);
assert_eq!(
fixture.read(".make-call"),
"built\n",
"make did not run in the tree"
);
// The image check passed, which is why the failure is qemu's rather than
// the image's.
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(
stderr.contains("could not run `qemu-system-x86_64`"),
"the build did not get as far as qemu: {stderr}"
);
}
#[test]
fn qemu_runs_with_the_arguments_the_plan_describes() {
// What qemu was handed, captured without needing a real qemu: the stub is
// reached through `PATH`, so the binary under test cannot tell.
let fixture = fixture_with_image();
fixture.write_script(
"bin/qemu-system-x86_64",
"#!/bin/sh\necho \"argv: $*\"\nexit 1\n",
);
let bin = fixture.join("bin");
let output = fixture.cli_env(
&[
"kernel",
"test",
"--no-build",
"--qemu-arg",
"-m",
"--qemu-arg",
"1G",
],
&[("PATH", bin.to_str().expect("the stub bin directory"))],
);
assert_eq!(
output.status.code(),
Some(1),
"a failing qemu must not look clean: {}",
String::from_utf8_lossy(&output.stdout)
);
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("-kernel ") && stdout.contains("arch/x86/boot/bzImage"),
"{stdout}"
);
assert!(
stdout.contains("-append console=ttyS0"),
"the console is what makes a boot readable: {stdout}"
);
assert!(stdout.contains("-nographic"), "{stdout}");
assert!(stdout.contains("-m 1G"), "the pass-through args: {stdout}");
assert!(
stdout.contains("qemu exited with status 1"),
"the exit status was flattened: {stdout}"
);
}
+45
View File
@@ -122,3 +122,48 @@ fn real_get_maintainer_fills_the_to_and_cc_lines() {
);
assert!(stdout.contains("Dry-OK"), "{stdout}");
}
#[test]
#[ignore = "needs SPECTRAL_LIVE=1 and a network"]
fn live_bugzilla_answers_with_one_issue() {
// The only test that talks to bugzilla.kernel.org. It asserts the shape of
// the answer rather than which bug came back, which is what a live API can
// be held to.
assert_eq!(
std::env::var("SPECTRAL_LIVE").as_deref(),
Ok("1"),
"run this with SPECTRAL_LIVE=1"
);
let fixture = Fixture::bare();
let output = fixture.cli(&["kernel", "quest", "--json"]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8_lossy(&output.stdout);
for field in ["\"id\"", "\"title\"", "\"component\"", "\"url\""] {
assert!(stdout.contains(field), "{field} is missing from {stdout}");
}
assert!(
stdout.contains("https://bugzilla.kernel.org/show_bug.cgi?id="),
"{stdout}"
);
}
#[test]
#[ignore = "needs SPECTRAL_LIVE=1 and a network"]
fn live_bugzilla_names_a_filter_that_matches_nothing() {
assert_eq!(
std::env::var("SPECTRAL_LIVE").as_deref(),
Ok("1"),
"run this with SPECTRAL_LIVE=1"
);
let fixture = Fixture::bare();
let output = fixture.cli(&["kernel", "quest", "--filter", "zzq-no-such-word-zzq"]);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("no open bug matched"), "{stderr}");
}
+464
View File
@@ -0,0 +1,464 @@
//! Patch series, driven through the built binary against a fixture tree.
//!
//! Every test name here carries the verb, so the plan's own command
//! `cargo test --locked series` selects exactly this file's tests.
mod common;
use std::path::{Path, PathBuf};
use common::Fixture;
/// The blurb the tests write into a cover letter.
const BLURB: &str = "Port the re-arm to v2\n\nThis replaces the v1 series after review.";
/// A tree with two commits on a branch of its own: the smallest thing that is
/// really a set rather than a patch.
fn fixture_with_two_commits() -> Fixture {
let fixture = Fixture::kernel_tree();
fixture.work_on_a_branch();
fixture.write("drivers/foo/bar.c", common::EDITED);
fixture.commit_all("foo: return a + 1");
fixture.write(
"drivers/foo/bar.c",
"static int foo(int a)\n{\n\treturn a + 2;\n}\n",
);
fixture.commit_all("foo: return a + 2");
fixture
}
/// The directory a series was written into, as `create` printed it.
fn create_series(fixture: &Fixture, extra: &[&str]) -> PathBuf {
let mut args = vec!["patch", "create", "xhci-series", "--range", "master..HEAD"];
args.extend_from_slice(extra);
let output = fixture.cli(&args);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
PathBuf::from(String::from_utf8_lossy(&output.stdout).trim())
}
/// What is in a directory, sorted, so two listings can be compared.
fn listing(dir: &Path) -> Vec<String> {
let mut names: Vec<String> = std::fs::read_dir(dir)
.expect("read the series directory")
.map(|entry| {
entry
.expect("an entry")
.file_name()
.to_string_lossy()
.into_owned()
})
.collect();
names.sort();
names
}
/// The patches in a directory, in name order.
fn patches(dir: &Path) -> Vec<String> {
listing(dir)
.into_iter()
.filter(|name| name.ends_with(".patch"))
.collect()
}
fn read(dir: &Path, name: &str) -> String {
std::fs::read_to_string(dir.join(name)).expect("read a file in the series directory")
}
#[test]
fn a_series_gets_the_names_and_the_cover_letter_format_patch_gives_it() {
let fixture = fixture_with_two_commits();
let dir = create_series(&fixture, &["--cover-letter", BLURB]);
assert_eq!(
dir,
fixture.home().join(".spectral/patches/xhci-series"),
"the series belongs in a directory of its own"
);
let names = listing(&dir);
assert_eq!(names.len(), 4, "{names:?}");
assert_eq!(names[0], "0000-cover-letter.patch");
assert!(names[1].starts_with("0001-"), "{names:?}");
assert!(names[2].starts_with("0002-"), "{names:?}");
assert_eq!(names[3], "series.toml", "the sidecar says what this is");
// The sidecar records the whole range as hashes, the revision, and the
// files in the order they are sent. The range matters as much as the base:
// a re-roll rebuilt as `base..HEAD` would pick up later commits.
let manifest = read(&dir, "series.toml");
let master = fixture.git(&["rev-parse", "master"]);
let head = fixture.git(&["rev-parse", "HEAD"]);
assert!(
manifest.contains(&format!("base = \"{master}\"")),
"the base is a hash: {manifest}"
);
assert!(
manifest.contains(&format!("head = \"{head}\"")),
"the head is a hash too: {manifest}"
);
assert!(manifest.contains("version = 1"), "{manifest}");
assert!(manifest.contains("0000-cover-letter.patch"), "{manifest}");
// The blurb landed, and the template that send-email refuses is gone.
let cover = read(&dir, "0000-cover-letter.patch");
assert!(
cover.contains("Subject: [PATCH 0/2] Port the re-arm to v2"),
"{cover}"
);
assert!(cover.contains("This replaces the v1 series"), "{cover}");
assert!(!cover.contains("*** BLURB HERE ***"), "{cover}");
assert!(!cover.contains("*** SUBJECT HERE ***"), "{cover}");
}
#[test]
fn a_series_goes_out_in_one_send_email_invocation() {
let fixture = fixture_with_two_commits();
create_series(&fixture, &["--cover-letter", BLURB]);
let output = fixture.cli(&["patch", "submit", "xhci-series", "--dry-run"]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8_lossy(&output.stdout);
let command = stdout
.lines()
.find(|line| line.contains("send-email --dry-run"))
.unwrap_or_else(|| panic!("no command line in {stdout}"));
for name in ["0000-cover-letter.patch", "0001-", "0002-"] {
assert!(command.contains(name), "{name} is not in {command}");
}
let cover_at = command.find("0000-cover-letter").expect("the cover letter");
let first_at = command.find("0001-").expect("the first patch");
assert!(
cover_at < first_at,
"the cover letter is sent first: {command}"
);
assert!(stdout.contains("Dry-OK"), "{stdout}");
// One lookup over the whole set: a series has one recipient list.
let call = fixture.read(".getmaintainer-call");
assert_eq!(
call.matches("argv:").count(),
1,
"the lookup ran more than once: {call}"
);
assert!(call.contains("0000-cover-letter.patch"), "{call}");
assert!(call.contains("0001-"), "{call}");
}
#[test]
fn a_series_without_a_blurb_is_refused_by_send_email() {
// No --cover-letter, so format-patch's template subject is still in the
// file and send-email refuses the set. The refusal has to reach the user.
let fixture = fixture_with_two_commits();
create_series(&fixture, &[]);
let output = fixture.cli(&["patch", "submit", "xhci-series", "--dry-run"]);
assert_eq!(output.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("Refusing to send"), "{stderr}");
assert!(stderr.contains("exit 25"), "{stderr}");
}
#[test]
fn series_reroll_moves_every_file_at_once_and_stays_idempotent() {
let fixture = fixture_with_two_commits();
let dir = create_series(&fixture, &["--cover-letter", BLURB]);
let before = listing(&dir);
// No -v: the next revision is inferred from the sidecar.
let output = fixture.cli(&["patch", "update", "xhci-series"]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
assert_eq!(
String::from_utf8_lossy(&output.stdout).trim(),
dir.display().to_string()
);
let after = listing(&dir);
assert_eq!(
after.len(),
before.len(),
"the old revision was left beside the new one: {after:?}"
);
assert!(
patches(&dir).iter().all(|name| name.starts_with("v2-")),
"every patch moves together: {after:?}"
);
assert!(
after
.iter()
.any(|name| name == "v2-0000-cover-letter.patch")
);
assert!(read(&dir, "series.toml").contains("version = 2"));
assert!(
read(&dir, "v2-0000-cover-letter.patch").contains("This replaces the v1 series"),
"a re-roll threw the blurb away"
);
// The same revision again is a no-op, which is what keeps a double run
// from producing a v3.
let again = fixture.cli(&["patch", "update", "xhci-series", "-v", "2"]);
assert!(again.status.success());
assert_eq!(
String::from_utf8_lossy(&again.stdout).trim(),
dir.display().to_string()
);
let unchanged = listing(&dir);
assert_eq!(unchanged, after, "a second run at v2 changed the directory");
assert!(
!unchanged.iter().any(|name| name.starts_with("v3-")),
"{unchanged:?}"
);
// And the re-rolled set still sends, with the cover letter first.
let sent = fixture.cli(&["patch", "submit", "xhci-series", "--dry-run"]);
assert!(
sent.status.success(),
"{}",
String::from_utf8_lossy(&sent.stderr)
);
assert!(String::from_utf8_lossy(&sent.stdout).contains("Dry-OK"));
}
#[test]
fn a_rerolled_series_holds_exactly_one_revision() {
let fixture = fixture_with_two_commits();
let dir = create_series(&fixture, &["--cover-letter", BLURB]);
// Straight to v3, so the files that must go are the unversioned ones.
assert!(
fixture
.cli(&["patch", "update", "xhci-series", "-v", "3"])
.status
.success()
);
assert!(
fixture
.cli(&["patch", "update", "xhci-series", "-v", "4"])
.status
.success()
);
let names = patches(&dir);
assert_eq!(names.len(), 3, "{names:?}");
assert!(
names.iter().all(|name| name.starts_with("v4-")),
"{names:?}"
);
}
#[test]
fn series_reroll_does_not_pick_up_a_commit_that_landed_after_create() {
// The range a series was created for is the range it re-rolls. Rebuilding
// it as `base..HEAD` would quietly mail commits nobody asked to send.
let fixture = fixture_with_two_commits();
let dir = create_series(&fixture, &["--cover-letter", BLURB]);
assert_eq!(patches(&dir).len(), 3);
fixture.write(
"drivers/foo/bar.c",
"static int foo(int a)\n{\n\treturn a + 3;\n}\n",
);
fixture.commit_all("foo: return a + 3");
let output = fixture.cli(&["patch", "update", "xhci-series"]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let names = patches(&dir);
assert_eq!(
names.len(),
3,
"a commit that landed after create joined the set: {names:?}"
);
assert!(
!names.iter().any(|name| name.contains("return-a-3")),
"{names:?}"
);
assert_eq!(
read(&dir, "series.toml").matches("version = 2").count(),
1,
"{}",
read(&dir, "series.toml")
);
}
#[test]
fn series_reroll_tolerates_a_patch_that_is_already_gone() {
// A missing file is the state the re-roll wanted, not a failure it should
// report forever while leaving the sidecar describing a revision that is
// no longer on disk.
let fixture = fixture_with_two_commits();
let dir = create_series(&fixture, &["--cover-letter", BLURB]);
let doomed = patches(&dir)
.into_iter()
.find(|name| name.starts_with("0002-"))
.expect("the second patch");
std::fs::remove_file(dir.join(&doomed)).expect("remove a patch by hand");
let output = fixture.cli(&["patch", "update", "xhci-series"]);
assert!(
output.status.success(),
"a stale name stopped the re-roll: {}",
String::from_utf8_lossy(&output.stderr)
);
let names = patches(&dir);
assert_eq!(names.len(), 3, "{names:?}");
assert!(
names.iter().all(|name| name.starts_with("v2-")),
"{names:?}"
);
assert!(read(&dir, "series.toml").contains("version = 2"));
// And it is ready to send, which is the thing the old failure made
// impossible: the sidecar and the directory agree again.
let sent = fixture.cli(&["patch", "submit", "xhci-series", "--dry-run"]);
assert!(
sent.status.success(),
"{}",
String::from_utf8_lossy(&sent.stderr)
);
}
#[test]
fn series_create_refuses_an_empty_blurb_before_it_writes_anything() {
let fixture = fixture_with_two_commits();
let output = fixture.cli(&[
"patch",
"create",
"xhci-series",
"--range",
"master..HEAD",
"--cover-letter",
" ",
]);
assert_eq!(output.status.code(), Some(1));
assert!(
String::from_utf8_lossy(&output.stderr).contains("blurb is empty"),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let dir = fixture.home().join(".spectral/patches/xhci-series");
assert!(
!dir.exists(),
"files with no sidecar beside them were left behind: {:?}",
listing(&dir)
);
}
#[test]
fn a_directory_that_is_not_a_series_still_sends_its_patches() {
// No sidecar: the directory is read as a pile of patches, in name order,
// which is the order format-patch numbered them in.
let fixture = fixture_with_two_commits();
let dir = fixture.home().join("hand-made");
std::fs::create_dir_all(&dir).expect("create the directory");
fixture.git(&[
"format-patch",
"-o",
dir.to_str().expect("the directory path"),
"master..HEAD",
]);
let output = fixture.cli(&[
"patch",
"submit",
dir.to_str().expect("the directory path"),
"--dry-run",
]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("0001-") && stdout.contains("0002-"),
"{stdout}"
);
}
#[test]
fn a_series_wants_a_range_and_a_range_that_is_not_one_is_named() {
let fixture = fixture_with_two_commits();
let not_a_range = fixture.cli(&["patch", "create", "x", "--range", "master"]);
assert_eq!(not_a_range.status.code(), Some(1));
let stderr = String::from_utf8_lossy(&not_a_range.stderr);
assert!(stderr.contains("is not a range"), "{stderr}");
let orphan_blurb = fixture.cli(&["patch", "create", "x", "--cover-letter", "hi"]);
assert_eq!(orphan_blurb.status.code(), Some(1));
assert!(
String::from_utf8_lossy(&orphan_blurb.stderr).contains("belongs to a series"),
"{}",
String::from_utf8_lossy(&orphan_blurb.stderr)
);
}
#[test]
fn create_needs_a_name_when_no_series_range_is_given() {
let fixture = fixture_with_two_commits();
let output = fixture.cli(&["patch", "create"]);
assert_eq!(output.status.code(), Some(1));
assert!(
String::from_utf8_lossy(&output.stderr).contains("wants a NAME"),
"{}",
String::from_utf8_lossy(&output.stderr)
);
}
#[test]
fn a_series_with_no_name_lands_in_the_patch_directory_itself() {
// NAME names the series directory, so leaving it off means the patch
// directory is the series directory.
let fixture = fixture_with_two_commits();
let output = fixture.cli(&[
"patch",
"create",
"--range",
"master..HEAD",
"--cover-letter",
BLURB,
]);
assert!(
output.status.success(),
"{}",
String::from_utf8_lossy(&output.stderr)
);
let patches_dir = fixture.home().join(".spectral/patches");
assert_eq!(
String::from_utf8_lossy(&output.stdout).trim(),
patches_dir.display().to_string()
);
assert!(patches_dir.join("series.toml").is_file());
assert!(patches_dir.join("0000-cover-letter.patch").is_file());
}