Read from Cargo.toml whether a crate is published (#3507)

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<Self> {
+        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::<DocumentMut>()?,
+        })
+    }
+
+    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,
+        );
+    }
+}

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<Package>,
 }
 
-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<Self> {
-        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::<DocumentMut>()?,
-        })
-    }
-
-    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]

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 {

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;
         }
 

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<Vec<Value>>
 
     for package in Package::iter() {
         // Not all packages have documentation built:
-        if !package.is_published() {
+        if !package.is_published(workspace) {
             continue;
         }
 

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

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)