From 9781684ae6770a5d32cf21f8c83407578cc3c201 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Mon, 1 Dec 2025 16:16:12 +0100 Subject: [PATCH 1/4] xtask: Add semver checks for esp-rom-sys --- esp-rom-sys/Cargo.toml | 1 + xtask/src/semver_check.rs | 24 +++++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/esp-rom-sys/Cargo.toml b/esp-rom-sys/Cargo.toml index d81a81c43a..014ac75267 100644 --- a/esp-rom-sys/Cargo.toml +++ b/esp-rom-sys/Cargo.toml @@ -13,6 +13,7 @@ license = "MIT OR Apache-2.0" links = "esp_rom_sys" [package.metadata.espressif] +semver-checked = true forever-unstable = true check-configs = [{ features = [] }] clippy-configs = [{ features = [] }] diff --git a/xtask/src/semver_check.rs b/xtask/src/semver_check.rs index d3e40a85ef..17613d29be 100644 --- a/xtask/src/semver_check.rs +++ b/xtask/src/semver_check.rs @@ -7,6 +7,7 @@ use std::{ use anyhow::{Context, Error}; use cargo_semver_checks::{Check, GlobalConfig, ReleaseType, Rustdoc}; use esp_metadata::{Chip, Config}; +use toml_edit::{Item, Value}; use crate::{Package, cargo::CargoArgsBuilder, commands::checker::download_baselines}; @@ -52,7 +53,6 @@ pub fn minimum_update( let mut cfg = GlobalConfig::new(); cfg.set_log_level(Some(log::Level::Info)); let result = semver_check.check_release(&mut cfg)?; - log::info!("Result {:?}", result); let mut min_required_update = ReleaseType::Patch; for (_, report) in result.crate_reports() { @@ -65,6 +65,28 @@ pub fn minimum_update( } } + let forever_unstable = package + .toml() + .espressif_metadata() + .and_then(|metadata| metadata.get("forever-unstable")) + .map(|item| match item { + Item::Value(Value::Boolean(b)) => *b.value(), + Item::Value(_) => { + log::warn!("Invalid value for 'forever-unstable' in metadata - must be a boolean"); + true + } + _ => false, + }) + .unwrap_or(false); + + if forever_unstable && min_required_update == ReleaseType::Major { + log::warn!( + "Downgrading required bump from Minor to Patch for unstable package: {}", + package + ); + min_required_update = ReleaseType::Minor; + } + Ok(min_required_update) } From 5475876f7826a1ef26a6979c68d1aea759e09c17 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Mon, 8 Dec 2025 10:43:09 +0100 Subject: [PATCH 2/4] Add tests and add is_forever_unstable method to the Package --- xtask/src/commands/release/execute_plan.rs | 27 ++--- xtask/src/commands/release/plan.rs | 20 +--- xtask/src/lib.rs | 13 +++ xtask/src/semver_check.rs | 117 +++++++++++++++------ 4 files changed, 104 insertions(+), 73 deletions(-) diff --git a/xtask/src/commands/release/execute_plan.rs b/xtask/src/commands/release/execute_plan.rs index 7d8d698135..b92bf8abb9 100644 --- a/xtask/src/commands/release/execute_plan.rs +++ b/xtask/src/commands/release/execute_plan.rs @@ -4,7 +4,6 @@ use anyhow::{Context, Result, bail, ensure}; use clap::Args; use esp_metadata::Chip; use strum::IntoEnumIterator; -use toml_edit::{Item, Value}; use crate::{ cargo::CargoToml, @@ -66,26 +65,12 @@ pub fn execute_plan(workspace: &Path, args: ApplyPlanArgs) -> Result<()> { ); } - if let Some(metadata) = package.espressif_metadata() - && let Some(Item::Value(forever_unstable)) = metadata.get("forever_unstable") - { - // Special case: some packages are perma-unstable, meaning they won't ever have - // a stable release. For these packages, we always use a - // patch release. - let forever_unstable = if let Value::Boolean(forever_unstable) = forever_unstable { - *forever_unstable.value() - } else { - log::warn!("Invalid value for 'forever_unstable' in metadata - must be a boolean"); - true - }; - - if forever_unstable && step.bump != VersionBump::Patch { - bail!( - "Cannot bump perma-unstable package {} to a non-patch version", - step.package - ); - } - }; + if package.package.is_forever_unstable() && step.bump != VersionBump::Patch { + bail!( + "Cannot bump perma-unstable package {} to a non-patch version", + step.package + ); + } let new_version = update_package(&mut package, &step.bump, !args.no_dry_run)?; diff --git a/xtask/src/commands/release/plan.rs b/xtask/src/commands/release/plan.rs index a459976ee7..3237d4fa54 100644 --- a/xtask/src/commands/release/plan.rs +++ b/xtask/src/commands/release/plan.rs @@ -6,7 +6,6 @@ use clap::Args; use esp_metadata::Chip; use serde::{Deserialize, Serialize}; use strum::IntoEnumIterator; -use toml_edit::{Item, Value}; use crate::{ Package, @@ -135,24 +134,7 @@ pub fn plan(workspace: &Path, args: PlanArgs) -> Result<()> { let amount = if package.is_semver_checked() { min_package_update(workspace, package, &all_chips)? } else { - let forever_unstable = if let Some(metadata) = - package_tomls[&package].espressif_metadata() - && let Some(Item::Value(forever_unstable)) = metadata.get("forever-unstable") - { - // Special case: some packages are perma-unstable, meaning they won't ever have - // a stable release. For these packages, we always use a - // patch release. - if let Value::Boolean(forever_unstable) = forever_unstable { - *forever_unstable.value() - } else { - log::warn!( - "Invalid value for 'forever-unstable' in metadata - must be a boolean" - ); - true - } - } else { - false - }; + let forever_unstable = package_tomls[&package].package.is_forever_unstable(); if forever_unstable { ReleaseType::Patch diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index 21bde7e1b4..d394bfdaef 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -490,6 +490,19 @@ impl Package { crate::commands::generate_rom_symbols::generate_rom_symbols(&package_path, chip)?; } Ok(()) + fn is_forever_unstable(&self) -> bool { + match self + .toml() + .espressif_metadata() + .and_then(|m| m.get("forever-unstable")) + { + Some(Item::Value(Value::Boolean(b))) => *b.value(), + Some(Item::Value(_)) => { + log::warn!("Invalid value for 'forever-unstable' in metadata - must be a boolean"); + true + } + _ => false, + } } } diff --git a/xtask/src/semver_check.rs b/xtask/src/semver_check.rs index 17613d29be..120aa0424f 100644 --- a/xtask/src/semver_check.rs +++ b/xtask/src/semver_check.rs @@ -7,7 +7,6 @@ use std::{ use anyhow::{Context, Error}; use cargo_semver_checks::{Check, GlobalConfig, ReleaseType, Rustdoc}; use esp_metadata::{Chip, Config}; -use toml_edit::{Item, Value}; use crate::{Package, cargo::CargoArgsBuilder, commands::checker::download_baselines}; @@ -53,40 +52,15 @@ pub fn minimum_update( let mut cfg = GlobalConfig::new(); cfg.set_log_level(Some(log::Level::Info)); let result = semver_check.check_release(&mut cfg)?; + log::info!("Result {:?}", result); - let mut min_required_update = ReleaseType::Patch; - for (_, report) in result.crate_reports() { - if let Some(required_bump) = report.required_bump() { - let required_is_stricter = (min_required_update == ReleaseType::Patch) - || (required_bump == ReleaseType::Major); - if required_is_stricter { - min_required_update = required_bump; - } - } - } - - let forever_unstable = package - .toml() - .espressif_metadata() - .and_then(|metadata| metadata.get("forever-unstable")) - .map(|item| match item { - Item::Value(Value::Boolean(b)) => *b.value(), - Item::Value(_) => { - log::warn!("Invalid value for 'forever-unstable' in metadata - must be a boolean"); - true - } - _ => false, - }) - .unwrap_or(false); - - if forever_unstable && min_required_update == ReleaseType::Major { - log::warn!( - "Downgrading required bump from Minor to Patch for unstable package: {}", - package - ); - min_required_update = ReleaseType::Minor; - } + let required_bumps: Vec = result + .crate_reports() + .into_iter() + .filter_map(|(_, report)| report.required_bump()) + .collect(); + let min_required_update = required_bump(&required_bumps, package.is_forever_unstable()); Ok(min_required_update) } @@ -148,3 +122,80 @@ pub(crate) fn build_doc_json( .with_context(|| format!("Failed to run `cargo rustdoc` with {cargo_args:?}",))?; Ok(current_path) } + +fn required_bump(required_bumps: &[ReleaseType], forever_unstable: bool) -> ReleaseType { + let mut min_required_update = ReleaseType::Patch; + + for &required_bump in required_bumps { + let required_is_stricter = + (min_required_update == ReleaseType::Patch) || (required_bump == ReleaseType::Major); + + if required_is_stricter { + min_required_update = required_bump; + } + } + + if forever_unstable && min_required_update == ReleaseType::Major { + log::warn!("Downgrading required bump from Minor to Patch for unstable package",); + min_required_update = ReleaseType::Minor; + } + + min_required_update +} + +#[cfg(test)] +mod tests { + + use super::*; + + #[test] + // Semver-check requiring a major bump for a 0.x crate + fn major_bump_from_0x_to_0x_plus_1() { + let bumps = [ReleaseType::Major]; + + let result = required_bump(&bumps, false); + + assert_eq!(result, ReleaseType::Major); + } + + #[test] + // For crates >= 1.0.0, Major is still Major + fn major_bump_from_1x_to_2x() { + let bumps = [ReleaseType::Major]; + + let result = required_bump(&bumps, false); + + assert_eq!(result, ReleaseType::Major); + } + + #[test] + fn forever_unstable_downgrades_major_to_minor() { + let bumps = [ReleaseType::Major]; + + let result = required_bump(&bumps, true); + + assert_eq!( + result, + ReleaseType::Minor, + "forever-unstable packages must never require a major bump" + ); + } + + #[test] + fn minor_stays_minor() { + let bumps = [ReleaseType::Minor]; + + let result = required_bump(&bumps, false); + + assert_eq!(result, ReleaseType::Minor); + } + + #[test] + fn multiple_bumps_select_major() { + let bumps = [ReleaseType::Patch, ReleaseType::Minor, ReleaseType::Major]; + + let result = required_bump(&bumps, false); + + assert_eq!(result, ReleaseType::Major); + } +} From 830abf72af246426faf447b68eb64298df1ec713 Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Mon, 15 Dec 2025 12:31:36 +0100 Subject: [PATCH 3/4] Panic if we receive Minor/Major bump for forever-unstable package --- xtask/src/semver_check.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/xtask/src/semver_check.rs b/xtask/src/semver_check.rs index 120aa0424f..324abea2ee 100644 --- a/xtask/src/semver_check.rs +++ b/xtask/src/semver_check.rs @@ -135,9 +135,11 @@ fn required_bump(required_bumps: &[ReleaseType], forever_unstable: bool) -> Rele } } - if forever_unstable && min_required_update == ReleaseType::Major { - log::warn!("Downgrading required bump from Minor to Patch for unstable package",); - min_required_update = ReleaseType::Minor; + if forever_unstable && min_required_update != ReleaseType::Patch { + panic!( + "Forever-unstable package can only receive a patch release but received {:?}!", + min_required_update + ); } min_required_update @@ -169,16 +171,23 @@ mod tests { } #[test] - fn forever_unstable_downgrades_major_to_minor() { + #[should_panic( + expected = "Forever-unstable package can only receive a patch release but received Major" + )] + fn forever_unstable_downgrades_major_to_patch() { let bumps = [ReleaseType::Major]; let result = required_bump(&bumps, true); + } - assert_eq!( - result, - ReleaseType::Minor, - "forever-unstable packages must never require a major bump" - ); + #[test] + #[should_panic( + expected = "Forever-unstable package can only receive a patch release but received Minor" + )] + fn forever_unstable_downgrades_minor_to_patch() { + let bumps = [ReleaseType::Minor]; + + let result = required_bump(&bumps, true); } #[test] From a8e404df390ee8931ace80f3c208948dfd1ff6dd Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Fri, 19 Dec 2025 15:26:45 +0100 Subject: [PATCH 4/4] rebase --- xtask/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index d394bfdaef..96eb38cbc3 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -490,6 +490,9 @@ impl Package { crate::commands::generate_rom_symbols::generate_rom_symbols(&package_path, chip)?; } Ok(()) + } + + #[cfg(feature = "semver-checks")] fn is_forever_unstable(&self) -> bool { match self .toml()