cargo-rbmt: add API break whitelist feature

Add allow_breaking_changes configuration option to filter out intentional
breaking changes from API check failures.

Author: nyonson

cargo-rbmt/README.md

@@ -9,8 +9,10 @@ Maintainer tools for Rust-based projects in the Bitcoin domain. Built with [xshe
 - [Lint](#lint)
 - [Test](#test)
 - [Integration](#integration)
+- [Fuzz](#fuzz)
 - [Prerelease](#prerelease)
 - [Lock Files](#lock-files)
+- [API Checking](#api-checking)
 - [Workspace Integration](#workspace-integration)
   - [1. Install on system](#1-install-on-system)
   - [2. Add as a dev-dependency](#2-add-as-a-dev-dependency)
@@ -91,6 +93,10 @@ package = "bitcoind-tests"
 versions = ["29_0", "28_2", "27_2"]
 ```
 
+## Fuzz
+
+The `fuzz` command assumes there is a package in a workspace which defines fuzz targets.
+
 ## Prerelease
 
 The `prerelease` command performs readiness checks before releasing a package. By default, all packages are checked unless they explicitly opt-out.
@@ -135,7 +141,7 @@ When you specify `--lock-file`, the tool copies that lock file to `Cargo.lock` b
 
 ## API Checking
 
-The `api` command helps maintain API stability by generating public API snapshots and checking for breaking changes. It uses the [public-api](https://github.com/Enselic/cargo-public-api) crate to analyze a crate's public interface.
+The `api` command helps maintain API stability by generating public API snapshots and checking for breaking changes. The generated API files are stored in `api/<package-name>/`.
 
 ```bash
 cargo rbmt api
@@ -145,14 +151,30 @@ cargo rbmt api
 2. Validates that features are additive (enabling features only adds to the API, never removes).
 3. Checks for uncommitted changes to API files.
 
-The generated API files are stored in `api/<package-name>/`.
-
 ```bash
 cargo rbmt api --baseline v0.1.0
 ```
 
 Compares the current API against a baseline git reference (tag, branch, or commit) to detect breaking changes.
 
+### Whitelisting Breaking Changes
+
+The `api` command checks for two types of breaking changes.
+
+* **Additivity** compares a package's `no-features` API vs. its `all-features` API to ensure its features are additive.
+* **Semver** compares some baseline version of a package's API vs the current API to ensure no breaking changes.
+
+Sometimes breaking changes are intentional and safe (e.g., replacing a specific impl with a more general blanket impl for sealed traits). Configure the `[api]` section a your package's `rbmt.toml` to whitelist specific changes:
+
+```toml
+[api]
+# Items that can be removed or changed without triggering errors.
+allow_breaking_changes = [
+    "impl MyTrait for SpecificType",
+    "pub fn deprecated_function()",
+]
+```
+
 ## Workspace Integration
 
 `cargo-rbmt` can simply be installed globally on a system or added as a dev-dependency to a package.

cargo-rbmt/src/api.rs

@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use std::fs;
 use std::path::{Path, PathBuf};
 
+use serde::Deserialize;
 use xshell::Shell;
 
 use crate::{environment, quiet_cmd, toolchain};
@@ -59,6 +60,38 @@ impl FeatureConfig {
     }
 }
 
+/// API configuration loaded from rbmt.toml.
+#[derive(Debug, Deserialize, Default)]
+#[serde(default)]
+struct Config {
+    api: ApiConfig,
+}
+
+/// API-specific configuration for a package.
+#[derive(Debug, Deserialize, Default)]
+#[serde(default)]
+struct ApiConfig {
+    /// List of API items that are allowed to be removed or changed without causing
+    /// semver or additivity check failures.
+    allow_breaking_changes: Vec<String>,
+}
+
+impl ApiConfig {
+    /// Load API configuration from the package directory.
+    fn load(_sh: &Shell, package_dir: &Path) -> Result<Self, Box<dyn std::error::Error>> {
+        let config_path = package_dir.join(environment::CONFIG_FILE_PATH);
+
+        if !config_path.exists() {
+            // Return empty config if file doesn't exist.
+            return Ok(Self { allow_breaking_changes: Vec::new() });
+        }
+
+        let contents = fs::read_to_string(&config_path)?;
+        let config: Config = toml::from_str(&contents)?;
+        Ok(config.api)
+    }
+}
+
 /// Run the API check task.
 ///
 /// This command checks for changes to the public API of workspace packages by generating
@@ -131,6 +164,44 @@ fn get_package_apis(
     Ok(apis)
 }
 
+/// Check for breaking changes in a diff, respecting the whitelist.
+///
+/// Returns a list of breaking change descriptions (empty if no breaks).
+fn check_for_breaking_changes(
+    diff: &public_api::diff::PublicApiDiff,
+    allow_breaking: &[String],
+) -> Vec<String> {
+    let mut breaking = Vec::new();
+
+    for item in &diff.removed {
+        let item_str = item.to_string();
+        if allow_breaking.contains(&item_str) {
+            environment::quiet_println(&format!(
+                "Warning: Allowing removal of '{}' (whitelisted)",
+                item_str
+            ));
+        } else {
+            breaking.push(format!("Removed: {}", item_str));
+        }
+    }
+
+    for change in &diff.changed {
+        let old_str = change.old.to_string();
+        let new_str = change.new.to_string();
+
+        if allow_breaking.contains(&old_str) || allow_breaking.contains(&new_str) {
+            environment::quiet_println(&format!(
+                "Warning: Allowing change to '{}' (whitelisted)",
+                old_str
+            ));
+        } else {
+            breaking.push(format!("Changed:\n    old: {}\n    new: {}", old_str, new_str));
+        }
+    }
+
+    breaking
+}
+
 /// Check API files for all packages.
 ///
 /// For each package, generates public API files for different feature configurations,
@@ -160,24 +231,14 @@ fn check_apis(
 
         let diff = public_api::diff::PublicApiDiff::between(no_features, all_features);
 
-        if !diff.removed.is_empty() || !diff.changed.is_empty() {
-            eprintln!("Non-additive features detected in {}:", package_name);
+        let config = ApiConfig::load(sh, package_dir)?;
+        let breaking_changes = check_for_breaking_changes(&diff, &config.allow_breaking_changes);
 
-            if !diff.removed.is_empty() {
-                eprintln!("  Items removed when enabling features:");
-                for item in &diff.removed {
-                    eprintln!("    - {}", item);
-                }
-            }
-
-            if !diff.changed.is_empty() {
-                eprintln!("  Items changed when enabling features:");
-                for item in &diff.changed {
-                    eprintln!("    - old: {}", item.old);
-                    eprintln!("      new: {}", item.new);
-                }
+        if !breaking_changes.is_empty() {
+            eprintln!("Non-additive features detected in {}:", package_name);
+            for change in breaking_changes {
+                eprintln!("  {}", change);
             }
-
             return Err("Non-additive features detected".into());
         }
     }
@@ -240,7 +301,7 @@ fn check_semver(
     quiet_cmd!(sh, "git switch {current_ref}").run()?;
 
     // Check for breaking changes in each package.
-    for package_name in package_info.iter().map(|(name, _)| name) {
+    for (package_name, package_dir) in package_info {
         let Some(mut baseline) = baseline_apis.remove(package_name) else {
             environment::quiet_println(&format!(
                 "Warning: Package '{}' not found in baseline - skipping comparison",
@@ -257,14 +318,22 @@ fn check_semver(
             continue;
         };
 
+        let api_config = ApiConfig::load(sh, package_dir)?;
+
         for config in [FeatureConfig::None, FeatureConfig::Alloc, FeatureConfig::All] {
             let baseline_api = baseline.remove(&config).ok_or("Config not found in baseline")?;
             let current_api = current.remove(&config).ok_or("Config not found in current")?;
 
             let diff = public_api::diff::PublicApiDiff::between(baseline_api, current_api);
 
-            if !diff.removed.is_empty() || !diff.changed.is_empty() {
-                eprintln!("API changes detected in {} ({})", package_name, config.display_name());
+            let breaking_changes =
+                check_for_breaking_changes(&diff, &api_config.allow_breaking_changes);
+
+            if !breaking_changes.is_empty() {
+                eprintln!("Breaking changes in {} ({}):", package_name, config.display_name());
+                for change in breaking_changes {
+                    eprintln!("  {}", change);
+                }
                 return Err("Semver compatibility check failed: breaking changes detected".into());
             }
         }