From d66b1397a21fe70ac167c8aed9d34ca0f9055926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 21 May 2025 11:27:16 +0200 Subject: [PATCH] Read from Cargo.toml whether a crate is published (#3507) --- xtask/src/cargo.rs | 126 ++++++++++++++++++++- xtask/src/commands/release/bump_version.rs | 115 +------------------ xtask/src/commands/release/publish.rs | 23 ++-- xtask/src/commands/release/tag_releases.rs | 2 +- xtask/src/documentation.rs | 6 +- xtask/src/lib.rs | 11 +- xtask/src/main.rs | 2 +- 7 files changed, 152 insertions(+), 133 deletions(-) diff --git a/xtask/src/cargo.rs b/xtask/src/cargo.rs index e504c08e2..ff2b830c4 100644 --- a/xtask/src/cargo.rs +++ b/xtask/src/cargo.rs @@ -6,10 +6,11 @@ use std::{ process::{Command, Stdio}, }; -use anyhow::{bail, Result}; +use anyhow::{Context as _, Result, bail}; use serde::{Deserialize, Serialize}; +use toml_edit::{DocumentMut, Item}; -use crate::windows_safe_path; +use crate::{Package, windows_safe_path}; #[derive(Clone, Debug, PartialEq)] pub enum CargoAction { @@ -191,3 +192,124 @@ impl CargoArgsBuilder { args } } + +pub struct CargoToml<'a> { + pub workspace: &'a Path, + pub package: Package, + pub manifest: toml_edit::DocumentMut, +} + +const DEPENDENCY_KINDS: [&'static str; 3] = + ["dependencies", "dev-dependencies", "build-dependencies"]; + +impl<'a> CargoToml<'a> { + pub fn new(workspace: &'a Path, package: Package) -> Result { + let package_path = workspace.join(package.to_string()); + let manifest_path = package_path.join("Cargo.toml"); + if !manifest_path.exists() { + bail!( + "Could not find Cargo.toml for package {package} at {}", + manifest_path.display() + ); + } + + let manifest = std::fs::read_to_string(&manifest_path) + .with_context(|| format!("Could not read {}", manifest_path.display()))?; + + Ok(Self { + workspace, + package, + manifest: manifest.parse::()?, + }) + } + + pub fn is_published(&self) -> bool { + // Check if the package is published by looking for the `publish` key + // in the manifest. + let Item::Table(package) = &self.manifest["package"] else { + unreachable!("The package table is missing in the manifest"); + }; + + let Some(publish) = package.get("publish") else { + return true; + }; + + publish.as_bool().unwrap_or(true) + } + + pub fn package_path(&self) -> PathBuf { + self.workspace.join(self.package.to_string()) + } + + pub fn manifest_path(&self) -> PathBuf { + self.package_path().join("Cargo.toml") + } + + pub fn version(&self) -> &str { + self.manifest["package"]["version"] + .as_str() + .unwrap() + .trim() + .trim_matches('"') + } + + pub fn set_version(&mut self, version: &semver::Version) { + log::info!( + "Bumping version for package: {} ({} -> {version})", + self.package, + self.version(), + ); + self.manifest["package"]["version"] = toml_edit::value(version.to_string()); + } + + pub fn save(&self) -> Result<()> { + let manifest_path = self.manifest_path(); + std::fs::write(&manifest_path, self.manifest.to_string()) + .with_context(|| format!("Could not write {}", manifest_path.display()))?; + + Ok(()) + } + + /// Calls a callback for each table that contains dependencies. + /// + /// Callback arguments: + /// - `path`: The path to the table (e.g. `dependencies.package`) + /// - `dependency_kind`: The kind of dependency (e.g. `dependencies`, + /// `dev-dependencies`) + /// - `table`: The table itself + pub fn visit_dependencies( + &mut self, + mut handle_dependencies: impl FnMut(&str, &'static str, &mut toml_edit::Table), + ) { + fn recurse_dependencies( + path: String, + table: &mut toml_edit::Table, + handle_dependencies: &mut impl FnMut(&str, &'static str, &mut toml_edit::Table), + ) { + // Walk through tables recursively so that we can find *all* dependencies. + for (key, item) in table.iter_mut() { + if let Item::Table(table) = item { + let path = if path.is_empty() { + key.to_string() + } else { + format!("{path}.{key}") + }; + recurse_dependencies(path, table, handle_dependencies); + } + } + for dependency_kind in DEPENDENCY_KINDS { + let Some(Item::Table(table)) = table.get_mut(dependency_kind) else { + continue; + }; + + handle_dependencies(&path, dependency_kind, table); + } + } + + recurse_dependencies( + String::new(), + self.manifest.as_table_mut(), + &mut handle_dependencies, + ); + } +} diff --git a/xtask/src/commands/release/bump_version.rs b/xtask/src/commands/release/bump_version.rs index 4581fa4db..c46202f85 100644 --- a/xtask/src/commands/release/bump_version.rs +++ b/xtask/src/commands/release/bump_version.rs @@ -8,9 +8,9 @@ use anyhow::{Context, Result, bail}; use clap::Args; use semver::Prerelease; use strum::IntoEnumIterator; -use toml_edit::{DocumentMut, Item, TableLike, Value}; +use toml_edit::{Item, TableLike, Value}; -use crate::{Package, Version, changelog::Changelog}; +use crate::{Package, Version, cargo::CargoToml, changelog::Changelog}; #[derive(Debug, Args)] pub struct BumpVersionArgs { @@ -32,113 +32,6 @@ pub struct BumpVersionArgs { packages: Vec, } -struct CargoToml<'a> { - workspace: &'a Path, - package: Package, - manifest: toml_edit::DocumentMut, -} - -const DEPENDENCY_KINDS: [&'static str; 3] = - ["dependencies", "dev-dependencies", "build-dependencies"]; - -impl<'a> CargoToml<'a> { - fn new(workspace: &'a Path, package: Package) -> Result { - let package_path = workspace.join(package.to_string()); - let manifest_path = package_path.join("Cargo.toml"); - if !manifest_path.exists() { - bail!( - "Could not find Cargo.toml for package {package} at {}", - manifest_path.display() - ); - } - - let manifest = fs::read_to_string(&manifest_path) - .with_context(|| format!("Could not read {}", manifest_path.display()))?; - - Ok(Self { - workspace, - package, - manifest: manifest.parse::()?, - }) - } - - fn package_path(&self) -> PathBuf { - self.workspace.join(self.package.to_string()) - } - - fn manifest_path(&self) -> PathBuf { - self.package_path().join("Cargo.toml") - } - - fn version(&self) -> &str { - self.manifest["package"]["version"] - .as_str() - .unwrap() - .trim() - .trim_matches('"') - } - - fn set_version(&mut self, version: &semver::Version) { - log::info!( - "Bumping version for package: {} ({} -> {version})", - self.package, - self.version(), - ); - self.manifest["package"]["version"] = toml_edit::value(version.to_string()); - } - - fn save(&self) -> Result<()> { - let manifest_path = self.manifest_path(); - fs::write(&manifest_path, self.manifest.to_string()) - .with_context(|| format!("Could not write {}", manifest_path.display()))?; - - Ok(()) - } - - /// Calls a callback for each table that contains dependencies. - /// - /// Callback arguments: - /// - `path`: The path to the table (e.g. `dependencies.package`) - /// - `dependency_kind`: The kind of dependency (e.g. `dependencies`, - /// `dev-dependencies`) - /// - `table`: The table itself - fn visit_dependencies( - &mut self, - mut handle_dependencies: impl FnMut(&str, &'static str, &mut toml_edit::Table), - ) { - fn recurse_dependencies( - path: String, - table: &mut toml_edit::Table, - handle_dependencies: &mut impl FnMut(&str, &'static str, &mut toml_edit::Table), - ) { - // Walk through tables recursively so that we can find *all* dependencies. - for (key, item) in table.iter_mut() { - if let Item::Table(table) = item { - let path = if path.is_empty() { - key.to_string() - } else { - format!("{path}.{key}") - }; - recurse_dependencies(path, table, handle_dependencies); - } - } - for dependency_kind in DEPENDENCY_KINDS { - let Some(Item::Table(table)) = table.get_mut(dependency_kind) else { - continue; - }; - - handle_dependencies(&path, dependency_kind, table); - } - } - - recurse_dependencies( - String::new(), - self.manifest.as_table_mut(), - &mut handle_dependencies, - ); - } -} - pub fn bump_version(workspace: &Path, args: BumpVersionArgs) -> Result<()> { // Bump the version by the specified amount for each given package: for package in args.packages { @@ -152,7 +45,7 @@ pub fn bump_version(workspace: &Path, args: BumpVersionArgs) -> Result<()> { Ok(()) } -fn check_crate_before_bumping(manifest: &mut CargoToml) -> Result<()> { +fn check_crate_before_bumping(manifest: &mut CargoToml<'_>) -> Result<()> { // Collect errors into a vector to preserve order. let mut errors = Vec::new(); @@ -423,6 +316,8 @@ fn finalize_placeholders( #[cfg(test)] mod test { + use toml_edit::DocumentMut; + use super::*; #[test] diff --git a/xtask/src/commands/release/publish.rs b/xtask/src/commands/release/publish.rs index c61da666c..f48b55fb6 100644 --- a/xtask/src/commands/release/publish.rs +++ b/xtask/src/commands/release/publish.rs @@ -1,6 +1,6 @@ use std::path::Path; -use anyhow::{Result, bail}; +use anyhow::{Result, ensure}; use clap::Args; use crate::{Package, cargo::CargoArgsBuilder, windows_safe_path}; @@ -20,19 +20,16 @@ pub fn publish(workspace: &Path, args: PublishArgs) -> Result<()> { let package_name = args.package.to_string(); let package_path = windows_safe_path(&workspace.join(&package_name)); - use Package::*; - let mut publish_args = match args.package { - Examples | HilTest | QaTest => { - bail!( - "Invalid package '{}' specified, this package should not be published!", - args.package - ) - } + ensure!( + args.package.is_published(workspace), + "Invalid package '{}' specified, this package should not be published!", + args.package + ); - EspBacktrace | EspHal | EspHalEmbassy | EspIeee802154 | EspLpHal | EspPrintln - | EspRiscvRt | EspStorage | EspWifi | XtensaLxRt => vec!["--no-verify"], - - _ => vec![], + let mut publish_args = if args.package.has_chip_features() { + vec!["--no-verify"] + } else { + vec![] }; if !args.no_dry_run { diff --git a/xtask/src/commands/release/tag_releases.rs b/xtask/src/commands/release/tag_releases.rs index 1b67cd350..09330e02c 100644 --- a/xtask/src/commands/release/tag_releases.rs +++ b/xtask/src/commands/release/tag_releases.rs @@ -31,7 +31,7 @@ pub fn tag_releases(workspace: &Path, mut args: TagReleasesArgs) -> Result<()> { // If a package does not require documentation, this also means that it is not // published (maybe this function needs a better name), so we can skip tagging // it: - if !package.is_published() { + if !package.is_published(workspace) { continue; } diff --git a/xtask/src/documentation.rs b/xtask/src/documentation.rs index f0cb28e6a..cfc249eb4 100644 --- a/xtask/src/documentation.rs +++ b/xtask/src/documentation.rs @@ -38,7 +38,7 @@ pub fn build_documentation( for package in packages { // Not all packages need documentation built: - if !package.is_published() { + if !package.is_published(workspace) { continue; } @@ -308,7 +308,7 @@ pub fn build_documentation_index(workspace: &Path, packages: &mut [Package]) -> for package in packages { // Not all packages have documentation built: - if !package.is_published() { + if !package.is_published(workspace) { continue; } @@ -431,7 +431,7 @@ fn generate_documentation_meta_for_index(workspace: &Path) -> Result> for package in Package::iter() { // Not all packages have documentation built: - if !package.is_published() { + if !package.is_published(workspace) { continue; } diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index 330888160..daae48a45 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -87,9 +87,14 @@ impl Package { matches!(self, EspHal | EspLpHal | EspWifi | EspHalEmbassy) } - /// Should documentation be built for the package? - pub fn is_published(&self) -> bool { - !matches!(self, Package::Examples | Package::HilTest | Package::QaTest) + /// Should documentation be built for the package, and should the package be + /// published? + pub fn is_published(&self, workspace: &Path) -> bool { + // TODO: we should use some sort of cache instead of parsing the TOML every + // time, but for now this should be good enough. + let toml = + crate::cargo::CargoToml::new(workspace, *self).expect("Failed to parse Cargo.toml"); + toml.is_published() } /// Build on host diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 7847fe914..008cb999a 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -184,7 +184,7 @@ fn lint_packages(workspace: &Path, args: LintPackagesArgs) -> Result<()> { let mut packages = args.packages; packages.sort(); - for package in packages.iter().filter(|p| p.is_published()) { + for package in packages.iter().filter(|p| p.is_published(workspace)) { // Unfortunately each package has its own unique requirements for // building, so we need to handle each individually (though there // is *some* overlap)