From f59c55e89eb7cc9897338adc7ce7b393284eb44d Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 19 Dec 2024 11:49:33 -0800 Subject: [PATCH] [omicron-package] add a way to show what Cargo commands will be run (#7268) Add a way to see what commands will be run, without actually running them. As part of this, refactor `CargoPlan` and change its meaning slightly -- it now indicates the set of debug and release packages that would be built. In the future, we'll be able to use these structures to add lints around which features get built. --- Cargo.lock | 1 + package/Cargo.toml | 1 + package/src/bin/omicron-package.rs | 174 ++++++++++------------------- package/src/cargo_plan.rs | 172 ++++++++++++++++++++++++++++ package/src/lib.rs | 3 + 5 files changed, 238 insertions(+), 113 deletions(-) create mode 100644 package/src/cargo_plan.rs diff --git a/Cargo.lock b/Cargo.lock index 4b1809d3b8..05a7082aaa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7140,6 +7140,7 @@ dependencies = [ "ring 0.17.8", "semver 1.0.23", "serde", + "shell-words", "sled-hardware", "slog", "slog-async", diff --git a/package/Cargo.toml b/package/Cargo.toml index b63a5ed96f..ccc768bb8e 100644 --- a/package/Cargo.toml +++ b/package/Cargo.toml @@ -25,6 +25,7 @@ reqwest = { workspace = true, features = [ "rustls-tls" ] } ring.workspace = true semver.workspace = true serde.workspace = true +shell-words.workspace = true sled-hardware.workspace = true slog.workspace = true slog-async.workspace = true diff --git a/package/src/bin/omicron-package.rs b/package/src/bin/omicron-package.rs index cc4050cbce..6ea10ae27b 100644 --- a/package/src/bin/omicron-package.rs +++ b/package/src/bin/omicron-package.rs @@ -4,12 +4,13 @@ //! Utility for bundling target binaries as tarfiles. -use anyhow::{anyhow, bail, ensure, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; use clap::{Parser, Subcommand}; use futures::stream::{self, StreamExt, TryStreamExt}; use illumos_utils::{zfs, zone}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; +use omicron_package::cargo_plan::build_cargo_plan; use omicron_package::target::KnownTarget; use omicron_package::{parse, BuildCommand, DeployCommand, TargetCommand}; use omicron_zone_package::config::{Config as PackageConfig, PackageMap}; @@ -24,7 +25,7 @@ use slog::o; use slog::Drain; use slog::Logger; use slog::{info, warn}; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::env; use std::fs::create_dir_all; use std::io::Write; @@ -105,128 +106,59 @@ struct Args { subcommand: SubCommand, } -#[derive(Debug, Default)] -struct CargoPlan<'a> { - command: &'a str, - packages: BTreeSet<&'a String>, - bins: BTreeSet<&'a String>, - features: BTreeSet<&'a String>, - release: bool, -} +async fn do_show_cargo_commands(config: &Config) -> Result<()> { + let metadata = cargo_metadata::MetadataCommand::new().no_deps().exec()?; + let features = config.cargo_features(); + let cargo_plan = + build_cargo_plan(&metadata, config.packages_to_build(), &features)?; -impl<'a> CargoPlan<'a> { - async fn run(&self, log: &Logger) -> Result<()> { - if self.bins.is_empty() { - return Ok(()); - } + let release_command = cargo_plan.release.build_command("build"); + let debug_command = cargo_plan.debug.build_command("build"); - let mut cmd = Command::new("cargo"); - // We rely on the rust-toolchain.toml file for toolchain information, - // rather than specifying one within the packaging tool. - cmd.arg(self.command); - // We specify _both_ --package and --bin; --bin does not imply - // --package, and without any --package options Cargo unifies features - // across all workspace default members. See rust-lang/cargo#8157. - for package in &self.packages { - cmd.arg("--package").arg(package); - } - for bin in &self.bins { - cmd.arg("--bin").arg(bin); - } - if !self.features.is_empty() { - cmd.arg("--features").arg(self.features.iter().fold( - String::new(), - |mut acc, s| { - if !acc.is_empty() { - acc.push(' '); - } - acc.push_str(s); - acc - }, - )); - } - if self.release { - cmd.arg("--release"); - } - info!(log, "running: {:?}", cmd.as_std()); - let status = cmd - .status() - .await - .context(format!("Failed to run command: ({:?})", cmd))?; - if !status.success() { - bail!("Failed to build packages"); - } + print!("release command: "); + if let Some(command) = release_command { + println!("{}", command_to_string(&command)); + } else { + println!("(none)"); + } - Ok(()) + print!("debug command: "); + if let Some(command) = debug_command { + println!("{}", command_to_string(&command)); + } else { + println!("(none)"); } + + Ok(()) +} + +fn command_to_string(command: &Command) -> String { + // Use shell-words to join the command and arguments into a single string. + let mut v = vec![command + .as_std() + .get_program() + .to_str() + .expect("program is valid UTF-8")]; + v.extend( + command + .as_std() + .get_args() + .map(|arg| arg.to_str().expect("argument is valid UTF-8")), + ); + + shell_words::join(&v) } async fn do_for_all_rust_packages( config: &Config, command: &str, ) -> Result<()> { - // Collect a map of all of the workspace packages - let workspace = cargo_metadata::MetadataCommand::new().no_deps().exec()?; - let workspace_pkgs = workspace - .packages - .into_iter() - .filter_map(|package| { - workspace - .workspace_members - .contains(&package.id) - .then_some((package.name.clone(), package)) - }) - .collect::>(); + let metadata = cargo_metadata::MetadataCommand::new().no_deps().exec()?; + let features = config.cargo_features(); + let cargo_plan = + build_cargo_plan(&metadata, config.packages_to_build(), &features)?; - // Generate a list of all features we might want to request - let features = config - .target - .0 - .iter() - .map(|(name, value)| format!("{name}-{value}")) - .collect::>(); - - // We split the packages to be built into "release" and "debug" lists - let mut release = - CargoPlan { command, release: true, ..Default::default() }; - let mut debug = CargoPlan { command, release: false, ..Default::default() }; - - for (name, pkg) in config.packages_to_build().0 { - // If this is a Rust package, `name` (the map key) is the name of the - // corresponding Rust crate. - if let PackageSource::Local { rust: Some(rust_pkg), .. } = &pkg.source { - let plan = if rust_pkg.release { &mut release } else { &mut debug }; - // Add the package name to the plan - plan.packages.insert(name); - // Get the package metadata - let metadata = workspace_pkgs.get(name).with_context(|| { - format!("package '{name}' is not a workspace package") - })?; - // Add the binaries we want to build to the plan - let bins = metadata - .targets - .iter() - .filter_map(|target| target.is_bin().then_some(&target.name)) - .collect::>(); - for bin in &rust_pkg.binary_names { - ensure!( - bins.contains(bin), - "bin target '{bin}' does not belong to package '{name}'" - ); - plan.bins.insert(bin); - } - // Add all features we want to request to the plan - plan.features.extend( - features - .iter() - .filter(|feature| metadata.features.contains_key(*feature)), - ); - } - } - - release.run(&config.log).await?; - debug.run(&config.log).await?; - Ok(()) + cargo_plan.run(command, &config.log).await } async fn do_check(config: &Config) -> Result<()> { @@ -1051,6 +983,19 @@ impl Config { } filtered_packages } + + /// Return a list of all possible Cargo features that could be requested for + /// the packages being built. + /// + /// Out of these, the features that actually get requested are determined by + /// which features are available for the list of packages being built. + fn cargo_features(&self) -> Vec { + self.target + .0 + .iter() + .map(|(name, value)| format!("{name}-{value}")) + .collect::>() + } } #[tokio::main] @@ -1142,6 +1087,9 @@ async fn main() -> Result<()> { ) .await?; } + SubCommand::Build(BuildCommand::ShowCargoCommands) => { + do_show_cargo_commands(&get_config()?).await?; + } SubCommand::Build(BuildCommand::Check) => { do_check(&get_config()?).await? } diff --git a/package/src/cargo_plan.rs b/package/src/cargo_plan.rs new file mode 100644 index 0000000000..1a32b199fb --- /dev/null +++ b/package/src/cargo_plan.rs @@ -0,0 +1,172 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use std::collections::BTreeMap; +use std::collections::BTreeSet; + +use anyhow::bail; +use anyhow::ensure; +use anyhow::Context; +use anyhow::Result; +use cargo_metadata::Metadata; +use omicron_zone_package::config::PackageMap; +use omicron_zone_package::package::PackageSource; +use slog::info; +use slog::Logger; +use tokio::process::Command; + +/// For a configuration, build a plan: the set of packages, binaries, and +/// features to operate on in release and debug modes. +pub fn build_cargo_plan<'a>( + metadata: &Metadata, + package_map: PackageMap<'a>, + features: &'a [String], +) -> Result> { + // Collect a map of all of the workspace packages + let workspace_pkgs = metadata + .packages + .iter() + .filter_map(|package| { + metadata + .workspace_members + .contains(&package.id) + .then_some((package.name.clone(), package)) + }) + .collect::>(); + + let mut release = CargoTargets::new(BuildKind::Release); + let mut debug = CargoTargets::new(BuildKind::Debug); + + for (name, pkg) in package_map.0 { + // If this is a Rust package, `name` (the map key) is the name of the + // corresponding Rust crate. + if let PackageSource::Local { rust: Some(rust_pkg), .. } = &pkg.source { + let plan = if rust_pkg.release { &mut release } else { &mut debug }; + // Add the package name to the plan + plan.packages.insert(name); + // Get the package metadata + let metadata = workspace_pkgs.get(name).with_context(|| { + format!("package '{name}' is not a workspace package") + })?; + // Add the binaries we want to build to the plan + let bins = metadata + .targets + .iter() + .filter_map(|target| target.is_bin().then_some(&target.name)) + .collect::>(); + for bin in &rust_pkg.binary_names { + ensure!( + bins.contains(bin), + "bin target '{bin}' does not belong to package '{name}'" + ); + plan.bins.insert(bin); + } + // Add all features we want to request to the plan + plan.features.extend( + features + .iter() + .filter(|feature| metadata.features.contains_key(*feature)), + ); + } + } + + Ok(CargoPlan { release, debug }) +} + +#[derive(Debug)] +pub struct CargoPlan<'a> { + pub release: CargoTargets<'a>, + pub debug: CargoTargets<'a>, +} + +impl CargoPlan<'_> { + pub async fn run(&self, command: &str, log: &Logger) -> Result<()> { + self.release.run(command, log).await?; + self.debug.run(command, log).await?; + Ok(()) + } +} + +/// A set of packages, binaries, and features to operate on. +#[derive(Debug)] +pub struct CargoTargets<'a> { + pub kind: BuildKind, + pub packages: BTreeSet<&'a String>, + pub bins: BTreeSet<&'a String>, + pub features: BTreeSet<&'a String>, +} + +impl CargoTargets<'_> { + fn new(kind: BuildKind) -> Self { + Self { + kind, + packages: BTreeSet::new(), + bins: BTreeSet::new(), + features: BTreeSet::new(), + } + } + + pub fn build_command(&self, command: &str) -> Option { + if self.bins.is_empty() { + return None; + } + + let mut cmd = Command::new("cargo"); + // We rely on the rust-toolchain.toml file for toolchain information, + // rather than specifying one within the packaging tool. + cmd.arg(command); + // We specify _both_ --package and --bin; --bin does not imply + // --package, and without any --package options Cargo unifies features + // across all workspace default members. See rust-lang/cargo#8157. + for package in &self.packages { + cmd.arg("--package").arg(package); + } + for bin in &self.bins { + cmd.arg("--bin").arg(bin); + } + if !self.features.is_empty() { + cmd.arg("--features").arg(self.features.iter().fold( + String::new(), + |mut acc, s| { + if !acc.is_empty() { + acc.push(' '); + } + acc.push_str(s); + acc + }, + )); + } + match self.kind { + BuildKind::Release => { + cmd.arg("--release"); + } + BuildKind::Debug => {} + } + + Some(cmd) + } + + pub async fn run(&self, command: &str, log: &Logger) -> Result<()> { + let Some(mut cmd) = self.build_command(command) else { + return Ok(()); + }; + + info!(log, "running: {:?}", cmd.as_std()); + let status = cmd + .status() + .await + .context(format!("Failed to run command: ({:?})", cmd))?; + if !status.success() { + bail!("Failed to build packages"); + } + + Ok(()) + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum BuildKind { + Release, + Debug, +} diff --git a/package/src/lib.rs b/package/src/lib.rs index b37c1774fd..9d58f476b2 100644 --- a/package/src/lib.rs +++ b/package/src/lib.rs @@ -5,6 +5,7 @@ use clap::Subcommand; use serde::de::DeserializeOwned; use thiserror::Error; +pub mod cargo_plan; pub mod dot; pub mod target; @@ -130,6 +131,8 @@ pub enum BuildCommand { /// The version to be stamped onto the package. version: semver::Version, }, + /// Show the Cargo commands that would be run to build the packages. + ShowCargoCommands, /// Checks the packages specified in a manifest, without building them. Check, }