Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 95 additions & 21 deletions cargo-soothfast/src/changelog_config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! The `[changelog]` section of `soothfast.toml`.
//! The `[changelog]` and `[changelog.icons]` tables of `soothfast.toml`.
//!
//! `soothfast.toml` is shared with the site, spec and gate engines, so this
//! parser skips every table it doesn't own.
Expand All @@ -9,45 +9,83 @@ use std::path::Path;
use soothfast_report::changelog::Icons;
use soothfast_site::toml::{TomlValue, logical_lines, parse_value};

/// Read `[changelog.icons]` from a directory's `soothfast.toml`. An absent
/// file just means the shipped icons.
pub fn load(dir: &Path) -> Result<Icons, String> {
/// Repo-level `report changelog` settings. CLI flags override these.
#[derive(Default, Debug)]
pub struct ChangelogConfig {
pub icons: Icons,
/// Cargo features the API surface is read under. Falls back to
/// `[gate] features` when absent.
pub features: Option<String>,
/// Packages `-p` defaults to.
pub packages: Vec<String>,
}

enum Table {
Changelog,
Icons,
Other,
}

/// Read `[changelog]` and `[changelog.icons]` from a directory's
/// `soothfast.toml`. An absent file just means defaults.
pub fn load(dir: &Path) -> Result<ChangelogConfig, String> {
let path = dir.join("soothfast.toml");
match std::fs::read_to_string(&path) {
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Icons::default()),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(ChangelogConfig::default()),
Err(e) => Err(format!("cannot read {}: {e}", path.display())),
Ok(text) => parse(&text).map_err(|e| format!("{}: {e}", path.display())),
}
}

/// Parse the `[changelog.icons]` table of a `soothfast.toml`.
pub fn parse(text: &str) -> Result<Icons, String> {
/// Parse the `[changelog]` and `[changelog.icons]` tables of a
/// `soothfast.toml`.
pub fn parse(text: &str) -> Result<ChangelogConfig, String> {
let mut cfg = ChangelogConfig::default();
let mut overrides = BTreeMap::new();
let mut in_icons = false;
let mut table = Table::Other;

for (lineno, line) in logical_lines(text) {
let line = line.as_str();
if let Some(inner) = line.strip_prefix('[').and_then(|l| l.strip_suffix(']')) {
in_icons = inner.trim() == "changelog.icons";
table = match inner.trim() {
"changelog" => Table::Changelog,
"changelog.icons" => Table::Icons,
_ => Table::Other,
};
continue;
}
if !in_icons {
if let Table::Other = table {
continue;
}

let Some((key, value)) = line.split_once('=') else {
return Err(format!("line {lineno}: expected `key = value`"));
};
let key = key.trim();
let value = parse_value(value.trim()).map_err(|e| format!("line {lineno}: {e}"))?;
let TomlValue::Str(icon) = value else {
return Err(format!(
"line {lineno}: `{}` under [changelog.icons] must be a string",
key.trim()
));
};
overrides.insert(key.trim().to_ascii_lowercase(), icon);
if let Table::Icons = table {
let TomlValue::Str(icon) = value else {
return Err(format!(
"line {lineno}: `{key}` under [changelog.icons] must be a string"
));
};
overrides.insert(key.to_ascii_lowercase(), icon);
} else {
set(&mut cfg, key, value).map_err(|e| format!("line {lineno}: {e}"))?;
}
}
Icons::new(overrides)

cfg.icons = Icons::new(overrides)?;
Ok(cfg)
}

fn set(cfg: &mut ChangelogConfig, key: &str, value: TomlValue) -> Result<(), String> {
match (key, value) {
("features", TomlValue::Str(s)) => cfg.features = Some(s),
("packages", TomlValue::StrArray(v)) => cfg.packages = v,
(k, _) => return Err(format!("unknown or mistyped `{k}` under [changelog]")),
}
Ok(())
}

#[cfg(test)]
Expand All @@ -57,13 +95,16 @@ mod tests {
#[test]
fn a_file_without_the_table_keeps_the_shipped_icons() {
let cfg = parse("[site]\nname = \"x\"\n").unwrap();
assert_eq!(format!("{cfg:?}"), format!("{:?}", Icons::default()));
assert_eq!(
format!("{:?}", cfg.icons),
format!("{:?}", Icons::default())
);
}

#[test]
fn declared_sections_are_taken_and_the_rest_stay_default() {
let icons = parse("[changelog.icons]\nfeatures = \"A\"\n").unwrap();
assert!(format!("{icons:?}").contains("features"));
let cfg = parse("[changelog.icons]\nfeatures = \"A\"\n").unwrap();
assert!(format!("{:?}", cfg.icons).contains("features"));
}

#[test]
Expand All @@ -77,4 +118,37 @@ mod tests {
let e = parse("[changelog.icons]\nfixes = 7\n").unwrap_err();
assert!(e.contains("must be a string"), "{e}");
}

#[test]
fn reads_features_and_packages() {
let cfg =
parse("[changelog]\nfeatures = \"full\"\npackages = [\"core\", \"server\"]\n").unwrap();
assert_eq!(cfg.features.as_deref(), Some("full"));
assert_eq!(cfg.packages, ["core", "server"]);
}

#[test]
fn the_two_changelog_tables_do_not_bleed_into_each_other() {
let text = "[changelog]\nfeatures = \"full\"\n\n[changelog.icons]\nfeatures = \"A\"\n";
let cfg = parse(text).unwrap();
assert_eq!(cfg.features.as_deref(), Some("full"));
assert!(format!("{:?}", cfg.icons).contains('A'));
}

#[test]
fn an_unknown_key_under_changelog_is_an_error() {
let e = parse("[changelog]\nnope = 1\n").unwrap_err();
assert!(e.contains("unknown or mistyped"), "{e}");
}

#[test]
fn a_mistyped_packages_is_an_error() {
assert!(parse("[changelog]\npackages = \"core\"\n").is_err());
}

#[test]
fn skips_tables_it_does_not_own() {
let text = "[gate]\nfeatures = \"bench-gate\"\n\n[changelog]\nfeatures = \"full\"\n";
assert_eq!(parse(text).unwrap().features.as_deref(), Some("full"));
}
}
24 changes: 22 additions & 2 deletions cargo-soothfast/src/gate_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::invoke::{self, CommonArgs};
#[derive(Default, Debug, PartialEq)]
pub struct GateConfig {
pub codegen_units: Option<String>,
/// Cargo features every `gate`, `measure` and `spec gen` build carries.
pub features: Option<String>,
}

/// Read `soothfast.toml` from a directory. An absent file just means
Expand All @@ -28,13 +30,19 @@ pub fn load(dir: &Path) -> Result<GateConfig, String> {

/// Fill in what the CLI did not set from the repo's `soothfast.toml`.
pub fn apply(common: &mut CommonArgs) -> Result<(), String> {
if common.codegen_units.is_some() {
if common.codegen_units.is_some() && common.features.is_some() {
return Ok(());
}
let Ok(root) = invoke::workspace_root() else {
return Ok(());
};
common.codegen_units = load(&root)?.codegen_units;
let cfg = load(&root)?;
if common.codegen_units.is_none() {
common.codegen_units = cfg.codegen_units;
}
if common.features.is_none() {
common.features = cfg.features;
}
Ok(())
}

Expand Down Expand Up @@ -66,6 +74,7 @@ fn set(cfg: &mut GateConfig, key: &str, value: TomlValue) -> Result<(), String>
match (key, value) {
("codegen-units", TomlValue::Int(n)) => cfg.codegen_units = Some(n.to_string()),
("codegen-units", TomlValue::Str(s)) => cfg.codegen_units = Some(s),
("features", TomlValue::Str(s)) => cfg.features = Some(s),
(k, _) => return Err(format!("unknown or mistyped `{k}` under [gate]")),
}
Ok(())
Expand All @@ -87,6 +96,17 @@ mod tests {
assert_eq!(cfg.codegen_units.as_deref(), Some("inherit"));
}

#[test]
fn reads_features() {
let cfg = parse("[gate]\nfeatures = \"bench-gate\"\n").unwrap();
assert_eq!(cfg.features.as_deref(), Some("bench-gate"));
}

#[test]
fn a_non_string_features_is_an_error() {
assert!(parse("[gate]\nfeatures = 3\n").is_err());
}

#[test]
fn skips_tables_it_does_not_own() {
let text = "[site]\nname = \"x\"\n\n[gate]\ncodegen-units = 2\n\n[[sdk]]\nspec = \"y\"\n";
Expand Down
62 changes: 52 additions & 10 deletions cargo-soothfast/src/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,18 +224,34 @@ fn measure_ref(a: &ReportArgs, refname: &str) -> Result<Option<Value>, String> {
}

fn changelog_cmd(args: &[String]) -> i32 {
let a = match parse(args) {
let mut a = match parse(args) {
Ok(a) => a,
Err(e) => return err(&e),
};
if a.pkg.is_empty() {
return err("report changelog needs -p PKG");
}

let root = match invoke::workspace_root() {
Ok(r) => r,
Err(e) => return err(&e.to_string()),
};
let cfg = match crate::changelog_config::load(&root) {
Ok(c) => c,
Err(e) => return err(&e),
};
if a.pkg.is_empty() {
a.pkg = cfg.packages;
}
if a.pkg.is_empty() {
return err("report changelog needs -p PKG");
}
let gate_cfg = match crate::gate_config::load(&root) {
Ok(g) => g,
Err(e) => return err(&e),
};
a.features = resolve_features(
a.features.take(),
cfg.features.as_deref(),
gate_cfg.features.as_deref(),
);
let path = a.out.clone().unwrap_or_else(|| root.join("CHANGELOG.md"));
let existing = std::fs::read_to_string(&path).unwrap_or_default();

Expand Down Expand Up @@ -292,13 +308,9 @@ fn changelog_cmd(args: &[String]) -> i32 {
},
None => Vec::new(),
};
let icons = match crate::changelog_config::load(&root) {
Ok(i) => i,
Err(e) => return err(&e),
};
let text = changelog::draft(&changelog::DraftInputs {
changes: &changes,
icons: &icons,
icons: &cfg.icons,
api: match &a.against_ref {
Some(refname) => changelog::ApiSection::Diff {
against: refname,
Expand All @@ -321,6 +333,16 @@ fn changelog_cmd(args: &[String]) -> i32 {
0
}

/// The features `report changelog` reads the API surface under: the command
/// line first, then `[changelog] features`, then `[gate] features`.
fn resolve_features(
cli: Option<String>,
changelog: Option<&str>,
gate: Option<&str>,
) -> Option<String> {
cli.or_else(|| changelog.or(gate).map(str::to_string))
}

/// Conventional-commit subjects merged since `refname`, newest first. Read
/// from git rather than a forge API: every merge lands as a squash whose
/// subject already carries its pull request number.
Expand Down Expand Up @@ -487,7 +509,7 @@ fn err(msg: &str) -> i32 {

#[cfg(test)]
mod tests {
use super::{changelog_already_cut, merge_changelog};
use super::{changelog_already_cut, merge_changelog, resolve_features};

#[test]
fn a_release_pr_still_diffing_against_the_previous_tag_is_already_cut() {
Expand Down Expand Up @@ -642,4 +664,24 @@ mod tests {
assert!(!merged.contains("never closed"));
assert!(merged.contains("new"));
}
#[test]
fn an_explicit_features_flag_beats_both_tables() {
let got = resolve_features(Some("cli".into()), Some("changelog"), Some("gate"));
assert_eq!(got.as_deref(), Some("cli"));
}

#[test]
fn changelog_features_beat_gate_features() {
let got = resolve_features(None, Some("changelog"), Some("gate"));
assert_eq!(got.as_deref(), Some("changelog"));
}

#[test]
fn gate_features_fill_in_when_nothing_else_does() {
assert_eq!(
resolve_features(None, None, Some("gate")).as_deref(),
Some("gate")
);
assert_eq!(resolve_features(None, None, None), None);
}
}
5 changes: 5 additions & 0 deletions cargo-soothfast/src/spec_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use serde_json::Value;
use soothfast_spec::dialect::{Info, Operation};
use soothfast_spec::schema::{self, Docs, TypeTable, route_sig};

use crate::gate_config;
use crate::invoke::{self, CommonArgs, Visibility};
use crate::spec_config::{self, Mode, SpecEntry};
use crate::workspace;
Expand Down Expand Up @@ -58,6 +59,10 @@ pub fn run(args: &[String]) -> i32 {
return 2;
}
}
if let Err(e) = gate_config::apply(&mut common) {
eprintln!("soothfast: {e}");
return 2;
}
let Some(pkg) = common.pkg.clone() else {
eprintln!("soothfast: spec gen requires -p PKG");
return 2;
Expand Down
16 changes: 16 additions & 0 deletions docs/gating.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,22 @@ A baseline saved before build stamps existed compares the same way, and says
to re-save it. `--against-ref` never hits that: both sides are measured fresh
in the same run.

## Features

A bench target declaring `required-features`, or a crate whose hot paths sit
behind a feature, needs `--features` on every run. `soothfast.toml` carries it
once, for `gate`, `measure` and `spec gen` alike:

```toml
[gate]
features = "bench-gate"
```

An explicit `--features` on the command line wins; the file only fills in what
the command line left unset. The action's `features` input sets the same thing
for CI, so a repository that configures the file gets one behaviour from a
hand-run `cargo soothfast gate` and from the action.

## Ratchets

<!-- soothfast:bind soothfast_measure::sweep::evaluate -->
Expand Down
15 changes: 15 additions & 0 deletions docs/reports.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,21 @@ features = "📐"
performance = "⏱"
```

The same file carries the flags `report changelog` would otherwise need on
every invocation:

```toml
[changelog]
features = "full"
packages = ["core", "server"]
```

`features` decides what the API surface diff can see: a public item behind a
feature that is not enabled never shows up in it, so too narrow a set shortens
the diff with nothing to say it did. It falls back to `[gate] features` when
absent. `packages` supplies `-p` when the command line gives none. An explicit
`--features` or `-p` still wins over either.

Below a rule sit the derived sections, evidence rather than narrative: the
public API diff against `v1.0`, and the measured movement past gate
thresholds. A section with nothing to report is omitted rather than shipped
Expand Down