rewrite check-workspace-deps as an xtask (#3710)

.github/workflows/check-workspace-deps.yml

@@ -11,9 +11,5 @@ jobs:
     runs-on: ubuntu-22.04
     steps:
       - uses: actions/[email protected]
-      - name: Install jq
-        run: sudo apt-get install -y jq
-      - name: Install toml-cli
-        run: cargo install [email protected]
       - name: Check Workspace Dependencies
-        run: ./tools/ci_check_workspace_deps.sh
+        run: cargo xtask check-workspace-deps

Cargo.toml

@@ -233,6 +233,7 @@ omicron-dev-tools = { path = "dev-tools" }
 omicron-gateway = { path = "gateway" }
 omicron-nexus = { path = "nexus" }
 omicron-package = { path = "package" }
+omicron-rpaths = { path = "rpaths" }
 omicron-sled-agent = { path = "sled-agent" }
 omicron-test-utils = { path = "test-utils" }
 omicron-zone-package = "0.8.3"

bootstore/Cargo.toml

@@ -5,8 +5,8 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
-[build-dependencies.omicron-rpaths]
-path = "../rpaths"
+[build-dependencies]
+omicron-rpaths.workspace = true
 
 [dependencies]
 bytes.workspace = true

dev-tools/Cargo.toml

@@ -4,8 +4,8 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
-[build-dependencies.omicron-rpaths]
-path = "../rpaths"
+[build-dependencies]
+omicron-rpaths.workspace = true
 
 [dependencies]
 anyhow.workspace = true

nexus/Cargo.toml

@@ -4,8 +4,8 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
-[build-dependencies.omicron-rpaths]
-path = "../rpaths"
+[build-dependencies]
+omicron-rpaths.workspace = true
 
 [dependencies]
 anyhow.workspace = true

nexus/db-model/Cargo.toml

@@ -4,8 +4,8 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
-[build-dependencies.omicron-rpaths]
-path = "../../rpaths"
+[build-dependencies]
+omicron-rpaths.workspace = true
 
 [dependencies]
 anyhow.workspace = true

nexus/db-queries/Cargo.toml

@@ -4,8 +4,8 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
-[build-dependencies.omicron-rpaths]
-path = "../../rpaths"
+[build-dependencies]
+omicron-rpaths.workspace = true
 
 [dependencies]
 anyhow.workspace = true

xtask/Cargo.toml

@@ -5,4 +5,7 @@ edition = "2021"
 
 [dependencies]
 anyhow.workspace = true
+camino.workspace = true
+cargo_toml = "0.15"
+cargo_metadata = "0.15"
 clap.workspace = true

xtask/src/main.rs

@@ -7,8 +7,11 @@
 //! See <https://github.com/matklad/cargo-xtask>.
 
 use anyhow::{bail, Context, Result};
+use camino::Utf8Path;
+use cargo_metadata::Metadata;
+use cargo_toml::{Dependency, Manifest};
 use clap::{Parser, Subcommand};
-use std::process::Command;
+use std::{collections::BTreeMap, process::Command};
 
 #[derive(Parser)]
 #[command(name = "cargo xtask", about = "Workspace-related developer tools")]
@@ -19,6 +22,9 @@ struct Args {
 
 #[derive(Subcommand)]
 enum Cmds {
+    /// Check that dependencies are not duplicated in any packages in the
+    /// workspace
+    CheckWorkspaceDeps,
     /// Run configured clippy checks
     Clippy,
 }
@@ -27,6 +33,7 @@ fn main() -> Result<()> {
     let args = Args::parse();
     match args.cmd {
         Cmds::Clippy => cmd_clippy(),
+        Cmds::CheckWorkspaceDeps => cmd_check_workspace_deps(),
     }
 }
 
@@ -70,3 +77,115 @@ fn cmd_clippy() -> Result<()> {
 
     Ok(())
 }
+
+fn cmd_check_workspace_deps() -> Result<()> {
+    // Ignore issues with "pq-sys".  See the omicron-rpaths package for details.
+    const EXCLUDED: &[&'static str] = &["pq-sys"];
+
+    // Collect a list of all packages used in any workspace package as a
+    // workspace dependency.
+    let mut workspace_dependencies = BTreeMap::new();
+
+    // Collect a list of all packages used in any workspace package as a
+    // NON-workspace dependency.
+    let mut non_workspace_dependencies = BTreeMap::new();
+
+    // Load information about the Cargo workspace.
+    let workspace = load_workspace()?;
+    let mut nwarnings = 0;
+    let mut nerrors = 0;
+
+    // Iterate the workspace packages and fill out the maps above.
+    for pkg_info in workspace.workspace_packages() {
+        let manifest_path = &pkg_info.manifest_path;
+        let manifest = read_cargo_toml(manifest_path)?;
+        for tree in [
+            &manifest.dependencies,
+            &manifest.dev_dependencies,
+            &manifest.build_dependencies,
+        ] {
+            for (name, dep) in tree {
+                if let Dependency::Inherited(inherited) = dep {
+                    if inherited.workspace {
+                        workspace_dependencies
+                            .entry(name.to_owned())
+                            .or_insert_with(Vec::new)
+                            .push(pkg_info.name.clone());
+
+                        if !inherited.features.is_empty() {
+                            eprintln!(
+                                "warning: package is used as a workspace dep \
+                                with extra features: {:?} (in {:?})",
+                                name, pkg_info.name,
+                            );
+                            nwarnings += 1;
+                        }
+
+                        continue;
+                    }
+                }
+
+                non_workspace_dependencies
+                    .entry(name.to_owned())
+                    .or_insert_with(Vec::new)
+                    .push(pkg_info.name.clone());
+            }
+        }
+    }
+
+    // Look for any packages that are used as both a workspace dependency and a
+    // non-workspace dependency.  Generally, the non-workspace dependency should
+    // be replaced with a workspace dependency.
+    for (pkgname, ws_examples) in &workspace_dependencies {
+        if let Some(non_ws_examples) = non_workspace_dependencies.get(pkgname) {
+            eprintln!(
+                "error: package is used as both a workspace dep and a \
+                non-workspace dep: {:?}",
+                pkgname
+            );
+            eprintln!("      workspace dep: {}", ws_examples.join(", "));
+            eprintln!("  non-workspace dep: {}", non_ws_examples.join(", "));
+            nerrors += 1;
+        }
+    }
+
+    // Look for any packages used as non-workspace dependencies by more than one
+    // workspace package.  These should generally be moved to a workspace
+    // dependency.
+    for (pkgname, examples) in
+        non_workspace_dependencies.iter().filter(|(pkgname, examples)| {
+            examples.len() > 1 && !EXCLUDED.contains(&pkgname.as_str())
+        })
+    {
+        eprintln!(
+            "error: package is used by multiple workspace packages without \
+            a workspace dependency: {:?}",
+            pkgname
+        );
+        eprintln!("  used in: {}", examples.join(", "));
+        nerrors += 1;
+    }
+
+    eprintln!(
+        "check-workspace-deps: errors: {}, warnings: {}",
+        nerrors, nwarnings
+    );
+
+    if nerrors != 0 {
+        bail!("errors with workspace dependencies");
+    }
+
+    Ok(())
+}
+
+fn read_cargo_toml(path: &Utf8Path) -> Result<Manifest> {
+    let bytes =
+        std::fs::read(path).with_context(|| format!("read {:?}", path))?;
+    Manifest::from_slice(&bytes).with_context(|| format!("parse {:?}", path))
+}
+
+fn load_workspace() -> Result<Metadata> {
+    cargo_metadata::MetadataCommand::new()
+        .exec()
+        .context("loading cargo metadata")
+}