Semver-Check (#3439)

* Semver-Check

* Clippy

* Improve

* Fix

* Adjust log level

* Add initial API baseline

* CI

* Fix

* Docs

* Debug CI

* Honor CARGO_TARGET_DIR

* Compressed API baseline

* Renaming

* Refactor

* Move code to commands mod

* Prepare for 1.85

* Address review comments

* Simplify

.cargo/config.toml

@@ -1,5 +1,6 @@
 [alias]
 xtask = "run --package xtask --"
-xdoc  = "run --package xtask --features=deploy-docs,preview-docs --"
+xdoc   = "run --package xtask --features=deploy-docs,preview-docs --"
-xfmt  = "xtask fmt-packages"
+xfmt   = "xtask fmt-packages"
-qa    = "xtask run example qa-test"
+qa     = "xtask run example qa-test"
+xcheck = "run --package xtask --features=semver-checks --"

.github/workflows/ci.yml

@@ -96,6 +96,10 @@ jobs:
         shell: bash
         run: cargo +${{ matrix.device.toolchain }} xtask ci ${{ matrix.device.soc }}
 
+      - name: Semver-Check
+        shell: bash
+        run: cargo +${{ matrix.device.toolchain }} xcheck semver-check --chips ${{ matrix.device.soc }} check
+
   extras:
     runs-on: macos-m1-self-hosted
 

esp-hal/Cargo.toml

@@ -11,6 +11,8 @@ repository    = "https://github.com/esp-rs/esp-hal"
 license       = "MIT OR Apache-2.0"
 links         = "esp-hal"
 
+exclude = [ "api-baseline", "MIGRATING-*", "CHANGELOG.md" ]
+
 [package.metadata.docs.rs]
 default-target = "riscv32imac-unknown-none-elf"
 features       = ["esp32c6"]

esp-hal/src/gpio/mod.rs

@@ -728,7 +728,7 @@ macro_rules! io_type {
     (Analog, $gpionum:literal) => {
         // FIXME: the implementation shouldn't be in the GPIO module
         #[cfg(any(esp32c2, esp32c3, esp32c6, esp32h2))]
-        #[cfg(any(doc, feature = "unstable"))]
+        #[cfg(feature = "unstable")]
         #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
         impl $crate::gpio::AnalogPin for paste::paste!( [<GPIO $gpionum>]<'_> ) {
             /// Configures the pin for analog mode.

esp-hal/src/lib.rs

@@ -576,14 +576,16 @@ pub struct Config {
     cpu_clock: CpuClock,
 
     /// Enable watchdog timer(s).
-    #[cfg(any(doc, feature = "unstable"))]
+    #[cfg(feature = "unstable")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
+    #[builder_lite(unstable)]
     watchdog: WatchdogConfig,
 
     /// PSRAM configuration.
-    #[cfg(any(doc, feature = "unstable"))]
+    #[cfg(feature = "unstable")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
     #[cfg(feature = "psram")]
+    #[builder_lite(unstable)]
     psram: psram::PsramConfig,
 }
 

esp-hal/src/peripheral.rs

@@ -51,7 +51,7 @@ macro_rules! peripherals {
                     #[doc = "**This API is marked as unstable** and is only available when the `unstable`
                             crate feature is enabled. This comes with no stability guarantees, and could be changed
                             or removed at any time."]
-                    #[cfg(any(doc, feature = "unstable"))]
+                    #[cfg(feature = "unstable")]
                     #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
                     pub $unstable_name: $unstable_name<'static>,
 
@@ -59,7 +59,7 @@ macro_rules! peripherals {
                     #[doc = "**This API is marked as unstable** and is only available when the `unstable`
                             crate feature is enabled. This comes with no stability guarantees, and could be changed
                             or removed at any time."]
-                    #[cfg(not(any(doc, feature = "unstable")))]
+                    #[cfg(not(feature = "unstable"))]
                     #[allow(unused)]
                     pub(crate) $unstable_name: $unstable_name<'static>,
                 )*

esp-hal/src/spi/master.rs

@@ -2469,10 +2469,10 @@ mod dma {
     }
 
     mod ehal1 {
-        #[cfg(any(doc, feature = "unstable"))]
+        #[cfg(feature = "unstable")]
         use embedded_hal::spi::{ErrorType, SpiBus};
 
-        #[cfg(any(doc, feature = "unstable"))]
+        #[cfg(feature = "unstable")]
         use super::*;
 
         #[instability::unstable]

esp-hal/src/spi/mod.rs

@@ -23,7 +23,7 @@ crate::unstable_module! {
 #[non_exhaustive]
 pub enum Error {
     /// Error occurred due to a DMA-related issue.
-    #[cfg(any(doc, feature = "unstable"))]
+    #[cfg(feature = "unstable")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
     #[allow(clippy::enum_variant_names, reason = "DMA is unstable")]
     DmaError(DmaError),
@@ -48,7 +48,7 @@ impl From<DmaError> for Error {
 }
 
 #[doc(hidden)]
-#[cfg(not(any(doc, feature = "unstable")))]
+#[cfg(not(feature = "unstable"))]
 impl From<DmaError> for Error {
     fn from(_value: DmaError) -> Self {
         Error::Unknown

esp-hal/src/uart.rs

@@ -450,7 +450,7 @@ pub struct UartRx<'d, Dm: DriverMode> {
 #[non_exhaustive]
 pub enum ConfigError {
     /// The requested baud rate is not achievable.
-    #[cfg(any(doc, feature = "unstable"))]
+    #[cfg(feature = "unstable")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
     UnachievableBaudrate,
 

xtask/Cargo.toml

@@ -30,6 +30,16 @@ reqwest = { version = "0.12.12", features = [
     "native-tls-vendored",
 ], optional = true }
 
+# This pulls a gazillion crates - don't include it by default
+cargo-semver-checks = { version = "0.41.0", optional = true }
+
+# THIS NEEDS TO MATCH THE rustfmt-json FORMAT - see https://github.com/rust-lang/rustdoc-types/blob/trunk/CHANGELOG.md
+rustdoc-types = { version = "0.35.0", optional = true }
+
+flate2 = { version = "1.1.1", optional = true }
+temp-file = { version = "0.1.9", optional = true }
+
 [features]
 deploy-docs  = ["dep:reqwest"]
 preview-docs = ["dep:opener", "dep:rocket"]
+semver-checks = [ "dep:cargo-semver-checks", "dep:rustdoc-types", "dep:flate2", "dep:temp-file" ]

xtask/src/commands/mod.rs

@@ -4,12 +4,13 @@ use anyhow::{Result, bail};
 use clap::Args;
 use esp_metadata::Chip;
 
-pub use self::{build::*, bump_version::*, run::*};
+pub use self::{build::*, bump_version::*, run::*, semver_check::*};
 use crate::{Package, cargo::CargoAction};
 
 mod build;
 mod bump_version;
 mod run;
+mod semver_check;
 
 // ----------------------------------------------------------------------------
 // Subcommand Arguments

xtask/src/commands/semver_check.rs

@@ -0,0 +1,145 @@
+use std::path::Path;
+
+use clap::{Args, Subcommand};
+use esp_metadata::Chip;
+use strum::IntoEnumIterator;
+
+use crate::Package;
+
+#[derive(Debug, Subcommand)]
+pub enum SemverCheckCmd {
+    GenerateBaseline,
+    Check,
+}
+
+#[derive(Debug, Args)]
+pub struct SemverCheckArgs {
+    #[command(subcommand)]
+    pub command: SemverCheckCmd,
+
+    /// Package(s) to target.
+    #[arg(long, value_enum, value_delimiter = ',', default_values_t = vec![Package::EspHal])]
+    pub packages: Vec<Package>,
+
+    /// Chip(s) to target.
+    #[arg(long, value_enum, value_delimiter = ',', default_values_t = Chip::iter())]
+    pub chips: Vec<Chip>,
+}
+
+pub fn semver_checks(workspace: &Path, args: SemverCheckArgs) -> anyhow::Result<()> {
+    #[cfg(not(feature = "semver-checks"))]
+    {
+        let _ = workspace;
+        let _ = args;
+
+        return Err(anyhow::anyhow!(
+            "Feature `semver-checks` is not enabled. Use the `xcheck` alias",
+        ));
+    }
+
+    #[cfg(feature = "semver-checks")]
+    match args.command {
+        SemverCheckCmd::GenerateBaseline => {
+            checker::generate_baseline(&workspace, args.packages, args.chips)
+        }
+        SemverCheckCmd::Check => {
+            checker::check_for_breaking_changes(&workspace, args.packages, args.chips)
+        }
+    }
+}
+
+#[cfg(feature = "semver-checks")]
+pub mod checker {
+    use std::{
+        fs,
+        io::Write,
+        path::{Path, PathBuf},
+    };
+
+    use cargo_semver_checks::ReleaseType;
+    use esp_metadata::Chip;
+
+    use crate::{
+        Package,
+        semver_check::{build_doc_json, minimum_update, remove_unstable_items},
+    };
+
+    pub fn generate_baseline(
+        workspace: &Path,
+        packages: Vec<Package>,
+        chips: Vec<Chip>,
+    ) -> Result<(), anyhow::Error> {
+        for package in packages {
+            log::info!("Generating API baseline for {package}");
+
+            for chip in &chips {
+                log::info!("Chip = {}", chip.to_string());
+                let package_name = package.to_string();
+                let package_path = crate::windows_safe_path(&workspace.join(&package_name));
+
+                let current_path = build_doc_json(package, chip, &package_path)?;
+
+                remove_unstable_items(&current_path)?;
+
+                let file_name = if package.chip_features_matter() {
+                    chip.to_string()
+                } else {
+                    "api".to_string()
+                };
+
+                let to_path = PathBuf::from(&package_path)
+                    .join(format!("api-baseline/{}.json.gz", file_name));
+                fs::create_dir_all(to_path.parent().unwrap())?;
+
+                log::debug!("Compress into {current_path:?}");
+                let mut encoder = flate2::write::GzEncoder::new(
+                    fs::File::create(to_path)?,
+                    flate2::Compression::default(),
+                );
+                encoder.write_all(&std::fs::read(current_path)?)?;
+
+                if !package.chip_features_matter() {
+                    break;
+                }
+            }
+        }
+
+        Ok(())
+    }
+
+    pub fn check_for_breaking_changes(
+        workspace: &Path,
+        packages: Vec<Package>,
+        chips: Vec<Chip>,
+    ) -> Result<(), anyhow::Error> {
+        let mut semver_incompatible_packages = Vec::new();
+
+        for package in packages {
+            log::info!("Semver-check API for {package}");
+
+            for chip in &chips {
+                let result = minimum_update(workspace, package, *chip)?;
+                log::info!("Required bump = {:?}", result);
+
+                if result == ReleaseType::Major {
+                    semver_incompatible_packages.push(package.to_string());
+                    // no need to check other chips for this package
+                    break;
+                }
+
+                if !package.chip_features_matter() {
+                    break;
+                }
+            }
+        }
+
+        if !semver_incompatible_packages.is_empty() {
+            Err(anyhow::anyhow!(
+                "Semver check failed - needs a major bump: {}",
+                semver_incompatible_packages.join(", ")
+            ))
+        } else {
+            Ok(())
+        }
+    }
+}

xtask/src/lib.rs

@@ -16,6 +16,9 @@ pub mod commands;
 pub mod documentation;
 pub mod firmware;
 
+#[cfg(feature = "semver-checks")]
+pub mod semver_check;
+
 #[derive(
     Debug,
     Clone,

xtask/src/main.rs

@@ -15,6 +15,8 @@ use xtask::{
     commands::*,
 };
 
+
+
 // ----------------------------------------------------------------------------
 // Command-line Interface
 
@@ -40,6 +42,8 @@ enum Cli {
     Publish(PublishArgs),
     /// Generate git tags for all new package releases.
     TagReleases(TagReleasesArgs),
+    /// Semver Checks
+    SemverCheck(SemverCheckArgs),
 }
 
 #[derive(Debug, Args)]
@@ -137,6 +141,7 @@ fn main() -> Result<()> {
         Cli::LintPackages(args) => lint_packages(&workspace, args),
         Cli::Publish(args) => publish(&workspace, args),
         Cli::TagReleases(args) => tag_releases(&workspace, args),
+        Cli::SemverCheck(args) => semver_checks(&workspace, args),
     }
 }
 

xtask/src/semver_check.rs

@@ -0,0 +1,220 @@
+use std::{
+    fs,
+    io::Write,
+    path::{Path, PathBuf},
+};
+
+use cargo_semver_checks::{Check, GlobalConfig, ReleaseType, Rustdoc};
+use esp_metadata::Chip;
+use rustdoc_types::ItemEnum;
+
+use crate::{Package, cargo::CargoArgsBuilder};
+
+/// Return the minimum required bump for the next release.
+/// Even if nothing changed this will be [ReleaseType::Patch]
+pub fn minimum_update(
+    workspace: &Path,
+    package: Package,
+    chip: Chip,
+) -> Result<ReleaseType, anyhow::Error> {
+    log::info!("Chip = {}", chip.to_string());
+
+    let package_name = package.to_string();
+    let package_path = crate::windows_safe_path(&workspace.join(&package_name));
+    let current_path = build_doc_json(package, &chip, &package_path)?;
+    remove_unstable_items(&current_path)?;
+
+    let file_name = if package.chip_features_matter() {
+        chip.to_string()
+    } else {
+        "api".to_string()
+    };
+
+    let baseline_path_gz =
+        PathBuf::from(&package_path).join(format!("api-baseline/{}.json.gz", file_name));
+    let baseline_path = temp_file::TempFile::new()?;
+    let buffer = Vec::new();
+    let mut decoder = flate2::write::GzDecoder::new(buffer);
+    decoder.write_all(&(fs::read(&baseline_path_gz)?))?;
+    fs::write(baseline_path.path(), decoder.finish()?)?;
+
+    let mut semver_check = Check::new(Rustdoc::from_path(current_path));
+    semver_check.set_baseline(Rustdoc::from_path(baseline_path.path()));
+    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;
+            }
+        }
+    }
+
+    Ok(min_required_update)
+}
+
+pub(crate) fn build_doc_json(
+    package: Package,
+    chip: &Chip,
+    package_path: &PathBuf,
+) -> Result<PathBuf, anyhow::Error> {
+    let target_dir = std::env::var("CARGO_TARGET_DIR");
+
+    let target_path = if let Ok(target) = target_dir {
+        PathBuf::from(target)
+    } else {
+        PathBuf::from(package_path).join("target")
+    };
+    let current_path = target_path
+        .join(chip.target())
+        .join("doc")
+        .join(format!("{}.json", package.to_string().replace("-", "_")));
+
+    std::fs::remove_file(&current_path).ok();
+    let features = if package.chip_features_matter() {
+        vec![chip.to_string(), "unstable".to_string()]
+    } else {
+        vec!["unstable".to_string()]
+    };
+
+    // always use `esp` toolchain so we don't have to deal with potentially
+    // different versions of the doc-json
+    let mut cargo_builder = CargoArgsBuilder::default()
+        .toolchain("esp")
+        .subcommand("rustdoc")
+        .features(&features)
+        .target(chip.target())
+        .arg("-Zunstable-options")
+        .arg("--lib")
+        .arg("--output-format=json");
+    cargo_builder = cargo_builder.arg("-Zbuild-std=alloc,core");
+    let cargo_args = cargo_builder.build();
+    log::debug!("{cargo_args:#?}");
+    crate::cargo::run(&cargo_args, package_path)?;
+    Ok(current_path)
+}
+
+pub(crate) fn remove_unstable_items(path: &Path) -> Result<(), anyhow::Error> {
+    // this leaves orphaned items! cargo-semver-checks seems to be fine with that
+    // however the json fmt is unstable - we might fail when using the "wrong"
+    // version of `rustdoc_types` here
+    //
+    // Hopefully this whole pre-processing is just a stop-gap solution until it's
+    // possible to generate docs for the stable-API only.
+
+    log::info!("{:?}", path);
+    let json_string = std::fs::read_to_string(path)?;
+    let mut krate: rustdoc_types::Crate = serde_json::from_str(&json_string)?;
+
+    let mut to_remove = vec![];
+
+    // first pass - just look for cfg-gated items
+    //
+    // the string to match depends on the rustfmt-json version!
+    // later version emit `#[<cfg>(...` instead
+    let cfg_gates = vec![
+        "#[cfg(any(doc, feature = \"unstable\"))]",
+        "#[cfg(feature = \"unstable\")]",
+    ];
+
+    for (id, item) in &mut krate.index {
+        if item
+            .attrs
+            .iter()
+            .any(|attr| cfg_gates.contains(&attr.as_str()))
+        {
+            to_remove.push(id.clone());
+        }
+    }
+
+    log::debug!("Items to remove {:?}", to_remove);
+
+    for id in &to_remove {
+        krate.index.remove(&id);
+        krate.paths.remove(&id);
+    }
+
+    for (_id, item) in &mut krate.index {
+        match &mut item.inner {
+            ItemEnum::Module(module) => {
+                module.items.retain(|id| !to_remove.contains(id));
+            }
+            ItemEnum::Struct(struct_) => {
+                struct_.impls.retain(|id| !to_remove.contains(id));
+
+                match &mut struct_.kind {
+                    rustdoc_types::StructKind::Unit => (),
+                    rustdoc_types::StructKind::Tuple(_ids) => (),
+                    rustdoc_types::StructKind::Plain {
+                        fields,
+                        has_stripped_fields: _,
+                    } => {
+                        for id in &to_remove {
+                            if let Some(found) = fields.iter().enumerate().find(|i| i.1 == id) {
+                                fields.remove(found.0);
+                            }
+                        }
+                    }
+                }
+
+                if struct_.impls.is_empty() {
+                    to_remove.push(_id.clone());
+                }
+            }
+            ItemEnum::Enum(enum_) => {
+                enum_.impls.retain(|id| !to_remove.contains(id));
+
+                enum_.variants.retain(|id| !to_remove.contains(id));
+
+                if enum_.impls.is_empty() {
+                    to_remove.push(_id.clone());
+                }
+            }
+            ItemEnum::Impl(impl_) => {
+                impl_.items.retain(|id| !to_remove.contains(id));
+
+                if impl_.items.is_empty() {
+                    to_remove.push(_id.clone());
+                }
+            }
+
+            // don't honor (because they either don't contain sub-items (= already handled in the
+            // first pass) or we currently don't use them)
+            //
+            // ItemEnum::Use(_)
+            // ItemEnum::Union(union)
+            // ItemEnum::StructField(_)
+            // ItemEnum::Variant(variant)
+            // ItemEnum::Function(function)
+            // ItemEnum::Trait(_)
+            // ItemEnum::TraitAlias(trait_alias)
+            // ItemEnum::TypeAlias(type_alias)
+            // ItemEnum::Constant {
+            // ItemEnum::Static(_)
+            // ItemEnum::ExternType =>
+            // ItemEnum::Macro(_)
+            // ItemEnum::ProcMacro(proc_macro)
+            // ItemEnum::Primitive(primitive)
+            // ItemEnum::AssocConst {
+            // ItemEnum::AssocType {
+            _ => (),
+        }
+    }
+
+    // if we added something more to remove (because the items are "empty" now -
+    // remove them, too)
+    for id in &to_remove {
+        krate.index.remove(&id);
+        krate.paths.remove(&id);
+    }
+
+    std::fs::write(path, serde_json::to_string(&krate)?)?;
+
+    Ok(())
+}