From abc7e3bb6172448167a1538b266d07f142c0cde1 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Tue, 9 Jul 2024 04:43:40 +0000 Subject: [PATCH 1/4] Add a gh-action and buildomat jobs to cargo check on no-default-features and feature-powerset Includes: - xtask wrapper around cargo-hack and options --- .cargo/config.toml | 1 + .github/buildomat/jobs/check-features.sh | 40 +++++++ .github/buildomat/jobs/clippy.sh | 2 +- .github/workflows/rust.yml | 36 +++++- dev-tools/xtask/src/check_features.rs | 134 +++++++++++++++++++++++ dev-tools/xtask/src/main.rs | 4 + 6 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 .github/buildomat/jobs/check-features.sh create mode 100644 dev-tools/xtask/src/check_features.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index c5b6fcd9d41..209d15c760b 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -8,6 +8,7 @@ # CI scripts: # - .github/buildomat/build-and-test.sh # - .github/buildomat/jobs/clippy.sh +# - .github/buildomat/jobs/check-features.sh # - .github/workflows/rust.yml # [build] diff --git a/.github/buildomat/jobs/check-features.sh b/.github/buildomat/jobs/check-features.sh new file mode 100644 index 00000000000..7e8ffc79859 --- /dev/null +++ b/.github/buildomat/jobs/check-features.sh @@ -0,0 +1,40 @@ +#!/bin/bash +#: +#: name = "check-features (helios)" +#: variety = "basic" +#: target = "helios-2.0" +#: rust_toolchain = true +#: output_rules = [] + +# Run cargo check on illumos with feature-specifics like `no-default-features` +# or `feature-powerset`. + +set -o errexit +set -o pipefail +set -o xtrace + +cargo --version +rustc --version + +# NOTE: This version should be in sync with the recommended version in +# ./dev-tools/xtask/src/check-features.rs. +CARGO_HACK_VERSION='0.6.28' + +# +# Set up our PATH for use with this workspace. +# +source ./env.sh + +banner prerequisites +ptime -m bash ./tools/install_builder_prerequisites.sh -y + +banner check +export CARGO_INCREMENTAL=0 +ptime -m cargo check --workspace --bins --tests --no-default-features +RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps --no-default-features + +# +# Check the feature set with the `cargo xtask check-features` command. +# +banner hack +ptime -m timeout 2h cargo xtask check-features --version "$CARGO_HACK_VERSION" --exclude-features image-trampoline,image-standard diff --git a/.github/buildomat/jobs/clippy.sh b/.github/buildomat/jobs/clippy.sh index 71aa04c907b..4040691b72a 100755 --- a/.github/buildomat/jobs/clippy.sh +++ b/.github/buildomat/jobs/clippy.sh @@ -10,7 +10,7 @@ # (that we want to check) is conditionally-compiled on illumos only. # # Note that `cargo clippy` includes `cargo check, so this ends up checking all -# of our code. +# of our (default) code. set -o errexit set -o pipefail diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 2ef27831083..cc307fec595 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -53,7 +53,7 @@ jobs: run: cargo run --bin omicron-package -- -t default check # Note that `cargo clippy` includes `cargo check, so this ends up checking all - # of our code. + # of our (default) code. clippy-lint: runs-on: ubuntu-22.04 env: @@ -82,6 +82,40 @@ jobs: - name: Run Clippy Lints run: cargo xtask clippy + check-features: + runs-on: ubuntu-22.04 + env: + CARGO_INCREMENTAL: 0 + CARGO_HACK_VERSION: 0.6.28 + steps: + # This repo is unstable and unnecessary: https://github.com/microsoft/linux-package-repositories/issues/34 + - name: Disable packages.microsoft.com repo + run: sudo rm -f /etc/apt/sources.list.d/microsoft-prod.list + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + with: + ref: ${{ github.event.pull_request.head.sha }} # see omicron#4461 + - uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2.7.3 + if: ${{ github.ref != 'refs/heads/main' }} + - name: Report cargo version + run: cargo --version + - name: Update PATH + run: source "./env.sh"; echo "PATH=$PATH" >> "$GITHUB_ENV" + - name: Print PATH + run: echo $PATH + - name: Print GITHUB_ENV + run: cat "$GITHUB_ENV" + - name: Install Pre-Requisites + run: ./tools/install_builder_prerequisites.sh -y + - name: Run `cargo check` for no-default-features + run: cargo check --workspace --bins --tests --no-default-features + # Uses manifest for install + - uses: taiki-e/install-action@v2 + with: + tool: cargo-hack@${{ env.CARGO_HACK_VERSION }} + - name: Run Check on Features (Feature-Powerset, No-Dev-Deps) + timeout-minutes: 120 # 2 hours + run: cargo xtask check-features --no-install --exclude-features image-trampoline,image-standard + # This is just a test build of docs. Publicly available docs are built via # the separate "rustdocs" repo. build-docs: diff --git a/dev-tools/xtask/src/check_features.rs b/dev-tools/xtask/src/check_features.rs new file mode 100644 index 00000000000..5f690e08502 --- /dev/null +++ b/dev-tools/xtask/src/check_features.rs @@ -0,0 +1,134 @@ +// 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/. + +//! Subcommand: cargo xtask check-features + +use anyhow::{bail, Context, Result}; +use clap::Parser; +use std::process::Command; + +/// The default version of `cargo-hack` to install. +/// We use a patch-floating version to avoid breaking the build when a new +/// version is released (locally). +const FLOAT_VERSION: &str = "~0.6.28"; + +#[derive(Parser)] +pub struct Args { + /// Features to exclude from the check. + #[clap(long)] + exclude_features: Option>, + /// Depth of the feature powerset to check. + #[clap(long)] + depth: Option, + /// Error format passed to `cargo hack check`. + #[clap(long, value_name = "FMT")] + message_format: Option, + /// Do not install `cargo-hack` before running the check. + #[clap(long, default_value_t = false)] + no_install: bool, + /// Version of `cargo-hack` to install. + #[clap(long)] + version: Option, +} + +/// Run `cargo hack check`. +pub fn run_cmd(args: Args) -> Result<()> { + if !args.no_install { + install_cargo_hack(args.version).unwrap(); + } + + let cargo = + std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); + let mut command = Command::new(&cargo); + + command.args(&["hack", "check"]); + + if let Some(features) = args.exclude_features { + let ex = format!("--exclude-features={}", features.join(",")); + command.arg(ex); + } + + if let Some(depth) = args.depth { + let depth = format!("depth={}", depth); + command.arg(depth); + } + + // Pass along the `--message-format` flag if it was provided. + if let Some(fmt) = args.message_format { + command.args(["--message-format", &fmt]); + } + + command + // Make sure we check everything. + .arg("--workspace") + .arg("--bins") + // We want to check the feature powerset. + .arg("--feature-powerset") + .arg("--no-dev-deps") + .arg("--exclude-no-default-features"); + + eprintln!( + "running: {:?} {}", + &cargo, + command + .get_args() + .map(|arg| format!("{:?}", arg.to_str().unwrap())) + .collect::>() + .join(" ") + ); + + let exit_status = command + .spawn() + .context("failed to spawn child process")? + .wait() + .context("failed to wait for child process")?; + + if !exit_status.success() { + bail!("check-features failed: {}", exit_status); + } + + Ok(()) +} + +/// Install `cargo-hack` at the specified version or the default version. +fn install_cargo_hack(version: Option) -> Result<()> { + let cargo = + std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); + + let mut command = Command::new(&cargo); + + if let Some(version) = version { + command.args(&["install", "cargo-hack", "--version", &version]); + } else { + command.args(&[ + "install", + "cargo-hack", + "--locked", + "--version", + FLOAT_VERSION, + ]); + } + + eprintln!( + "running: {:?} {}", + &cargo, + command + .get_args() + .map(|arg| format!("{:?}", arg.to_str().unwrap())) + .collect::>() + .join(" ") + ); + + let exit_status = command + .spawn() + .expect("failed to spawn child process") + .wait() + .expect("failed to wait for child process"); + + if !exit_status.success() { + bail!("cargo-hack install failed: {}", exit_status); + } + + Ok(()) +} diff --git a/dev-tools/xtask/src/main.rs b/dev-tools/xtask/src/main.rs index d0a61272a9f..0ea2332c317 100644 --- a/dev-tools/xtask/src/main.rs +++ b/dev-tools/xtask/src/main.rs @@ -10,6 +10,7 @@ use anyhow::{Context, Result}; use cargo_metadata::Metadata; use clap::{Parser, Subcommand}; +mod check_features; mod check_workspace_deps; mod clippy; mod download; @@ -38,6 +39,8 @@ enum Cmds { /// Run Argon2 hash with specific parameters (quick performance check) Argon2(external::External), + /// Check that all features are flagged correctly + CheckFeatures(check_features::Args), /// Check that dependencies are not duplicated in any packages in the /// workspace CheckWorkspaceDeps, @@ -91,6 +94,7 @@ async fn main() -> Result<()> { external.cargo_args(["--release"]).exec_example("argon2") } Cmds::Clippy(args) => clippy::run_cmd(args), + Cmds::CheckFeatures(args) => check_features::run_cmd(args), Cmds::CheckWorkspaceDeps => check_workspace_deps::run_cmd(), Cmds::Download(args) => download::run_cmd(args).await, Cmds::Openapi(external) => external.exec_bin("openapi-manager"), From 4abc5bbaa6142d03c3c68d46885354d99c948e42 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Fri, 12 Jul 2024 04:56:33 +0000 Subject: [PATCH 2/4] review: xtask updates on options/parameters around install-version, ci Includes: - xtask documentation on main readme --- .github/buildomat/jobs/check-features.sh | 14 +++--- .github/workflows/rust.yml | 4 +- README.adoc | 18 ++++++++ dev-tools/xtask/src/check_features.rs | 57 +++++++++++++++++------- 4 files changed, 64 insertions(+), 29 deletions(-) diff --git a/.github/buildomat/jobs/check-features.sh b/.github/buildomat/jobs/check-features.sh index 7e8ffc79859..3b79baa22d9 100644 --- a/.github/buildomat/jobs/check-features.sh +++ b/.github/buildomat/jobs/check-features.sh @@ -6,8 +6,7 @@ #: rust_toolchain = true #: output_rules = [] -# Run cargo check on illumos with feature-specifics like `no-default-features` -# or `feature-powerset`. +# Run the check-features `xtask` on illumos, testing compilation of feature combinations. set -o errexit set -o pipefail @@ -28,13 +27,10 @@ source ./env.sh banner prerequisites ptime -m bash ./tools/install_builder_prerequisites.sh -y -banner check -export CARGO_INCREMENTAL=0 -ptime -m cargo check --workspace --bins --tests --no-default-features -RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps --no-default-features - # # Check the feature set with the `cargo xtask check-features` command. # -banner hack -ptime -m timeout 2h cargo xtask check-features --version "$CARGO_HACK_VERSION" --exclude-features image-trampoline,image-standard +banner hack-check +export CARGO_INCREMENTAL=0 +ptime -m timeout 2h cargo xtask check-features --ci --install-version "$CARGO_HACK_VERSION" +RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps --no-default-features diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index cc307fec595..7ee558edbea 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -106,15 +106,13 @@ jobs: run: cat "$GITHUB_ENV" - name: Install Pre-Requisites run: ./tools/install_builder_prerequisites.sh -y - - name: Run `cargo check` for no-default-features - run: cargo check --workspace --bins --tests --no-default-features # Uses manifest for install - uses: taiki-e/install-action@v2 with: tool: cargo-hack@${{ env.CARGO_HACK_VERSION }} - name: Run Check on Features (Feature-Powerset, No-Dev-Deps) timeout-minutes: 120 # 2 hours - run: cargo xtask check-features --no-install --exclude-features image-trampoline,image-standard + run: cargo xtask check-features --ci # This is just a test build of docs. Publicly available docs are built via # the separate "rustdocs" repo. diff --git a/README.adoc b/README.adoc index 1ef4bd8601d..4e88cbab89d 100644 --- a/README.adoc +++ b/README.adoc @@ -112,6 +112,24 @@ cargo nextest run We check that certain system library dependencies are not leaked outside of their intended binaries via `cargo xtask verify-libraries` in CI. If you are adding a new dependency on a illumos/helios library it is recommended that you update xref:.cargo/xtask.toml[] with an allow list of where you expect the dependency to show up. For example some libraries such as `libnvme.so.1` are only available in the global zone and therefore will not be present in any other zone. This check is here to help us catch any leakage before we go to deploy on a rack. You can inspect a compiled binary in the target directory for what it requires by using `elfedit` - for example `elfedit -r -e 'dyn:tag NEEDED' /path/to/omicron/target/debug/sled-agent`. +=== Checking feature flag combinations + +To ensure that varying combinations of features compile, run `cargo xtask check-features`, which executes the https://github.com/taiki-e/cargo-hack[`cargo hack`] subcommand under the hood. This `xtask` is run in CI using the `--ci` parameter , which automatically exludes certain `image-*` features that purposefully cause compiler errors if set. + +If you don't have `cargo hack` installed locally, run the the `xtask` with the `install-version ` option, which will install it into your user's `.cargo` directory: + +[source,text] +---- +$ cargo xtask check-features --install-version 0.6.28 +---- + +To limit the max number of simultaneous feature flags combined for checking, run the `xtask` with the `--depth ` flag: + +[source,text] +---- +$ cargo xtask check-features --depth 2 +---- + === Rust packages in Omicron NOTE: The term "package" is overloaded: most programming languages and operating systems have their own definitions of a package. On top of that, Omicron bundles up components into our own kind of "package" that gets delivered via the install and update systems. These are described in the `package-manifest.toml` file in the root of the repo. In this section, we're just concerned with Rust packages. diff --git a/dev-tools/xtask/src/check_features.rs b/dev-tools/xtask/src/check_features.rs index 5f690e08502..9b5883e7c1d 100644 --- a/dev-tools/xtask/src/check_features.rs +++ b/dev-tools/xtask/src/check_features.rs @@ -6,52 +6,74 @@ use anyhow::{bail, Context, Result}; use clap::Parser; -use std::process::Command; +use std::{collections::HashSet, process::Command}; /// The default version of `cargo-hack` to install. /// We use a patch-floating version to avoid breaking the build when a new /// version is released (locally). const FLOAT_VERSION: &str = "~0.6.28"; +const CI_EXCLUDED_FEATURES: [&str; 2] = ["image-trampoline", "image-standard"]; + #[derive(Parser)] pub struct Args { + /// Run in CI mode, with a default set of features excluded. + #[clap(long, default_value_t = false)] + ci: bool, /// Features to exclude from the check. - #[clap(long)] + #[clap(long, value_name = "FEATURES")] exclude_features: Option>, /// Depth of the feature powerset to check. - #[clap(long)] + #[clap(long, value_name = "NUM")] depth: Option, /// Error format passed to `cargo hack check`. #[clap(long, value_name = "FMT")] message_format: Option, - /// Do not install `cargo-hack` before running the check. - #[clap(long, default_value_t = false)] - no_install: bool, /// Version of `cargo-hack` to install. - #[clap(long)] - version: Option, + #[clap(long, value_name = "VERSION")] + install_version: Option, } /// Run `cargo hack check`. pub fn run_cmd(args: Args) -> Result<()> { - if !args.no_install { - install_cargo_hack(args.version).unwrap(); + // Install `cargo-hack` if the `install-version` was specified. + if let Some(version) = args.install_version { + install_cargo_hack(Some(version))?; } let cargo = std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); let mut command = Command::new(&cargo); + // Add the `hack check` subcommand. command.args(&["hack", "check"]); - if let Some(features) = args.exclude_features { - let ex = format!("--exclude-features={}", features.join(",")); - command.arg(ex); + // Add the `--exclude-features` flag if we are running in CI mode. + if args.ci { + let ex = if let Some(mut features) = args.exclude_features { + // Extend the list of features to exclude with the CI defaults. + features.extend( + CI_EXCLUDED_FEATURES.into_iter().map(|s| s.to_string()), + ); + + // Remove duplicates. + let excludes = features.into_iter().collect::>(); + + excludes.into_iter().collect::>().join(",") + } else { + CI_EXCLUDED_FEATURES.join(",") + }; + + command.args(["--exclude-features", &ex]); + } else { + // Add "only" the `--exclude-features` flag if it was provided. + if let Some(features) = args.exclude_features { + command.args(["--exclude-features", &features.join(",")]); + } } if let Some(depth) = args.depth { - let depth = format!("depth={}", depth); - command.arg(depth); + command.args(&["--depth", &depth.to_string()]); } // Pass along the `--message-format` flag if it was provided. @@ -62,11 +84,12 @@ pub fn run_cmd(args: Args) -> Result<()> { command // Make sure we check everything. .arg("--workspace") + // We want to check the binaries. .arg("--bins") // We want to check the feature powerset. .arg("--feature-powerset") - .arg("--no-dev-deps") - .arg("--exclude-no-default-features"); + // We will not check the dev-dependencies, which should covered by tests. + .arg("--no-dev-deps"); eprintln!( "running: {:?} {}", From 3b5be0b4788c5e3fdc81e7d3fab2321c17efc118 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Sat, 13 Jul 2024 01:58:23 +0000 Subject: [PATCH 3/4] work with pre-built bins --- .github/buildomat/jobs/check-features.sh | 14 +-- .github/workflows/rust.yml | 12 +- README.adoc | 8 +- dev-tools/xtask/src/check_features.rs | 137 ++++++++++++++++------- dev-tools/xtask/src/download.rs | 77 +++++++++++++ tools/cargo_hack_checksum | 3 + tools/cargo_hack_version | 1 + 7 files changed, 192 insertions(+), 60 deletions(-) create mode 100644 tools/cargo_hack_checksum create mode 100644 tools/cargo_hack_version diff --git a/.github/buildomat/jobs/check-features.sh b/.github/buildomat/jobs/check-features.sh index 3b79baa22d9..4ba97ec02f9 100644 --- a/.github/buildomat/jobs/check-features.sh +++ b/.github/buildomat/jobs/check-features.sh @@ -4,7 +4,9 @@ #: variety = "basic" #: target = "helios-2.0" #: rust_toolchain = true -#: output_rules = [] +#: output_rules = [ +#: "/out/*", +#: ] # Run the check-features `xtask` on illumos, testing compilation of feature combinations. @@ -15,22 +17,18 @@ set -o xtrace cargo --version rustc --version -# NOTE: This version should be in sync with the recommended version in -# ./dev-tools/xtask/src/check-features.rs. -CARGO_HACK_VERSION='0.6.28' - # # Set up our PATH for use with this workspace. # source ./env.sh +export PATH="$PATH:$PWD/out/cargo-hack" banner prerequisites ptime -m bash ./tools/install_builder_prerequisites.sh -y # -# Check the feature set with the `cargo xtask check-features` command. +# Check feature combinations with the `cargo xtask check-features` command. # banner hack-check export CARGO_INCREMENTAL=0 -ptime -m timeout 2h cargo xtask check-features --ci --install-version "$CARGO_HACK_VERSION" -RUSTDOCFLAGS="--document-private-items -D warnings" ptime -m cargo doc --workspace --no-deps --no-default-features +ptime -m timeout 2h cargo xtask check-features --ci diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7ee558edbea..94d25e7dfa7 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -86,7 +86,6 @@ jobs: runs-on: ubuntu-22.04 env: CARGO_INCREMENTAL: 0 - CARGO_HACK_VERSION: 0.6.28 steps: # This repo is unstable and unnecessary: https://github.com/microsoft/linux-package-repositories/issues/34 - name: Disable packages.microsoft.com repo @@ -99,18 +98,17 @@ jobs: - name: Report cargo version run: cargo --version - name: Update PATH - run: source "./env.sh"; echo "PATH=$PATH" >> "$GITHUB_ENV" + run: | + set -x + export PATH="./out/cargo-hack:$PATH" + source "./env.sh"; echo "PATH=$PATH" >> "$GITHUB_ENV" - name: Print PATH run: echo $PATH - name: Print GITHUB_ENV run: cat "$GITHUB_ENV" - name: Install Pre-Requisites run: ./tools/install_builder_prerequisites.sh -y - # Uses manifest for install - - uses: taiki-e/install-action@v2 - with: - tool: cargo-hack@${{ env.CARGO_HACK_VERSION }} - - name: Run Check on Features (Feature-Powerset, No-Dev-Deps) + - name: Run Check on Feature Combinations (Feature-Powerset, No-Dev-Deps) timeout-minutes: 120 # 2 hours run: cargo xtask check-features --ci diff --git a/README.adoc b/README.adoc index 4e88cbab89d..2183f6e0faa 100644 --- a/README.adoc +++ b/README.adoc @@ -114,9 +114,13 @@ We check that certain system library dependencies are not leaked outside of thei === Checking feature flag combinations -To ensure that varying combinations of features compile, run `cargo xtask check-features`, which executes the https://github.com/taiki-e/cargo-hack[`cargo hack`] subcommand under the hood. This `xtask` is run in CI using the `--ci` parameter , which automatically exludes certain `image-*` features that purposefully cause compiler errors if set. +To ensure that varying combinations of features compile, run `cargo xtask check-features`, which executes the https://github.com/taiki-e/cargo-hack[`cargo hack`] subcommand under the hood. -If you don't have `cargo hack` installed locally, run the the `xtask` with the `install-version ` option, which will install it into your user's `.cargo` directory: +This `xtask` is run in CI using the `--ci` parameter , which automatically exludes certain `image-*` features that purposefully cause compiler errors if set and uses a pre-built binary. + +If `cargo hack` is not already installed in omicron's `out/` directory, a pre-built binary will be installed automatically depending on your operating system and architecture. + +You can also run the the `xtask` with the `install-version ` option, which will install the cargo subcommand into your user's `.cargo` directory: [source,text] ---- diff --git a/dev-tools/xtask/src/check_features.rs b/dev-tools/xtask/src/check_features.rs index 9b5883e7c1d..f4efd6791ce 100644 --- a/dev-tools/xtask/src/check_features.rs +++ b/dev-tools/xtask/src/check_features.rs @@ -4,15 +4,12 @@ //! Subcommand: cargo xtask check-features -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use clap::Parser; use std::{collections::HashSet, process::Command}; -/// The default version of `cargo-hack` to install. -/// We use a patch-floating version to avoid breaking the build when a new -/// version is released (locally). -const FLOAT_VERSION: &str = "~0.6.28"; - +const SUPPORTED_ARCHITECTURES: [&str; 1] = ["x86_64"]; const CI_EXCLUDED_FEATURES: [&str; 2] = ["image-trampoline", "image-standard"]; #[derive(Parser)] @@ -29,27 +26,31 @@ pub struct Args { /// Error format passed to `cargo hack check`. #[clap(long, value_name = "FMT")] message_format: Option, - /// Version of `cargo-hack` to install. + /// Version of `cargo-hack` to install. By default, we download a pre-built + /// version. #[clap(long, value_name = "VERSION")] install_version: Option, } /// Run `cargo hack check`. pub fn run_cmd(args: Args) -> Result<()> { - // Install `cargo-hack` if the `install-version` was specified. - if let Some(version) = args.install_version { - install_cargo_hack(Some(version))?; + // We cannot specify both `--ci` and `--install-version`, as the former + // implies we are using a pre-built version. + if args.ci && args.install_version.is_some() { + bail!("cannot specify --ci and --install-version together"); } let cargo = std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); + let mut command = Command::new(&cargo); // Add the `hack check` subcommand. command.args(&["hack", "check"]); - // Add the `--exclude-features` flag if we are running in CI mode. if args.ci { + install_prebuilt_cargo_hack(&cargo)?; + let ex = if let Some(mut features) = args.exclude_features { // Extend the list of features to exclude with the CI defaults. features.extend( @@ -64,8 +65,10 @@ pub fn run_cmd(args: Args) -> Result<()> { CI_EXCLUDED_FEATURES.join(",") }; + // Add the `--exclude-features` flag if we are running in CI mode. command.args(["--exclude-features", &ex]); } else { + install_cargo_hack(&cargo, args.install_version)?; // Add "only" the `--exclude-features` flag if it was provided. if let Some(features) = args.exclude_features { command.args(["--exclude-features", &features.join(",")]); @@ -91,48 +94,96 @@ pub fn run_cmd(args: Args) -> Result<()> { // We will not check the dev-dependencies, which should covered by tests. .arg("--no-dev-deps"); - eprintln!( - "running: {:?} {}", - &cargo, - command - .get_args() - .map(|arg| format!("{:?}", arg.to_str().unwrap())) - .collect::>() - .join(" ") - ); - - let exit_status = command - .spawn() - .context("failed to spawn child process")? - .wait() - .context("failed to wait for child process")?; - - if !exit_status.success() { - bail!("check-features failed: {}", exit_status); - } + exec(command) +} - Ok(()) +/// The supported operating systems. +enum Os { + Illumos, + Linux, + Mac, } -/// Install `cargo-hack` at the specified version or the default version. -fn install_cargo_hack(version: Option) -> Result<()> { - let cargo = - std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); +/// Get the current OS. +fn os_name() -> Result { + let os = match std::env::consts::OS { + "linux" => Os::Linux, + "macos" => Os::Mac, + "solaris" | "illumos" => Os::Illumos, + other => bail!("OS not supported: {other}"), + }; + Ok(os) +} - let mut command = Command::new(&cargo); +/// Get the path to the `out` directory. +fn out_dir() -> Utf8PathBuf { + if let Ok(omicron_dir) = std::env::var("OMICRON") { + Utf8Path::new(format!("{}/out/cargo-hack", omicron_dir).as_str()) + .to_path_buf() + } else { + Utf8Path::new("out/cargo-hack").to_path_buf() + } +} +/// Install `cargo-hack` if the `install-version` was specified; otherwise, +/// download a pre-built version if it's not already in our `out` directory. +fn install_cargo_hack(cargo: &str, version: Option) -> Result<()> { if let Some(version) = version { + let mut command = Command::new(cargo); + + eprintln!( + "installing cargo-hack at version {} to {}", + version, + env!("CARGO_HOME") + ); command.args(&["install", "cargo-hack", "--version", &version]); + exec(command) + } else if !out_dir().exists() { + install_prebuilt_cargo_hack(cargo) } else { - command.args(&[ - "install", - "cargo-hack", - "--locked", - "--version", - FLOAT_VERSION, - ]); + let out_dir = out_dir(); + eprintln!("cargo-hack found in {}", out_dir); + Ok(()) + } +} + +/// Download a pre-built version of `cargo-hack` to the `out` directory via the +/// download `xtask`. +fn install_prebuilt_cargo_hack(cargo: &str) -> Result<()> { + let mut command = Command::new(cargo); + + let out_dir = out_dir(); + eprintln!( + "cargo-hack not found in {}, downloading a pre-built version", + out_dir + ); + + let os = os_name()?; + match os { + Os::Illumos | Os::Linux | Os::Mac + if SUPPORTED_ARCHITECTURES.contains(&std::env::consts::ARCH) => + { + // Download the pre-built version of `cargo-hack` via our + // download `xtask`. + command.args(&["xtask", "download", "cargo-hack"]); + } + _ => { + bail!( + "cargo-hack is not pre-built for this os {} / arch {}", + std::env::consts::OS, + std::env::consts::ARCH + ); + } } + exec(command) +} + +/// Execute the command and check the exit status. +fn exec(mut command: Command) -> Result<()> { + let cargo = + std::env::var("CARGO").unwrap_or_else(|_| String::from("cargo")); + eprintln!( "running: {:?} {}", &cargo, diff --git a/dev-tools/xtask/src/download.rs b/dev-tools/xtask/src/download.rs index 2790a638a71..37c9b7be8a4 100644 --- a/dev-tools/xtask/src/download.rs +++ b/dev-tools/xtask/src/download.rs @@ -17,6 +17,7 @@ use std::io::Write; use std::os::unix::fs::PermissionsExt; use std::sync::OnceLock; use std::time::Duration; +use strum::Display; use strum::EnumIter; use strum::IntoEnumIterator; use tar::Archive; @@ -25,6 +26,9 @@ use tokio::process::Command; const BUILDOMAT_URL: &'static str = "https://buildomat.eng.oxide.computer/public/file"; +const CARGO_HACK_URL: &'static str = + "https://github.com/taiki-e/cargo-hack/releases/download"; + const RETRY_ATTEMPTS: usize = 3; /// What is being downloaded? @@ -44,6 +48,9 @@ enum Target { /// Download all targets All, + /// `cargo hack` binary + CargoHack, + /// Clickhouse binary Clickhouse, @@ -124,6 +131,7 @@ pub async fn run_cmd(args: DownloadArgs) -> Result<()> { Target::All => { bail!("We should have already filtered this 'All' target out?"); } + Target::CargoHack => downloader.download_cargo_hack().await, Target::Clickhouse => downloader.download_clickhouse().await, Target::Cockroach => downloader.download_cockroach().await, Target::Console => downloader.download_console().await, @@ -151,12 +159,19 @@ pub async fn run_cmd(args: DownloadArgs) -> Result<()> { Ok(()) } +#[derive(Display)] enum Os { Illumos, Linux, Mac, } +#[derive(Display)] +enum Arch { + X86_64, + Aarch64, +} + impl Os { fn env_name(&self) -> &'static str { match self { @@ -177,6 +192,15 @@ fn os_name() -> Result { Ok(os) } +fn arch() -> Result { + let arch = match std::env::consts::ARCH { + "x86_64" => Arch::X86_64, + "aarch64" => Arch::Aarch64, + other => bail!("Architecture not supported: {other}"), + }; + Ok(arch) +} + struct Downloader<'a> { log: Logger, @@ -432,6 +456,59 @@ async fn download_file_and_verify( } impl<'a> Downloader<'a> { + async fn download_cargo_hack(&self) -> Result<()> { + let os = os_name()?; + let arch = arch()?; + + let download_dir = self.output_dir.join("downloads"); + let destination_dir = self.output_dir.join("cargo-hack"); + + let checksums_path = self.versions_dir.join("cargo_hack_checksum"); + let [checksum] = get_values_from_file( + [&format!("CIDL_SHA256_{}", os.env_name())], + &checksums_path, + ) + .await?; + + let versions_path = self.versions_dir.join("cargo_hack_version"); + let version = tokio::fs::read_to_string(&versions_path) + .await + .context("Failed to read version from {versions_path}")?; + let version = version.trim(); + + let (platform, supported_arch) = match (os, arch) { + (Os::Illumos, Arch::X86_64) => ("unknown-illumos", "x86_64"), + (Os::Linux, Arch::X86_64) => ("unknown-linux-gnu", "x86_64"), + (Os::Linux, Arch::Aarch64) => ("unknown-linux-gnu", "aarch64"), + (Os::Mac, Arch::X86_64) => ("apple-darwin", "x86_64"), + (Os::Mac, Arch::Aarch64) => ("apple-darwin", "aarch64"), + (os, arch) => bail!("Unsupported OS/arch: {os}/{arch}"), + }; + + let tarball_filename = + format!("cargo-hack-{supported_arch}-{platform}.tar.gz"); + let tarball_url = + format!("{CARGO_HACK_URL}/v{version}/{tarball_filename}"); + + let tarball_path = download_dir.join(&tarball_filename); + + tokio::fs::create_dir_all(&download_dir).await?; + tokio::fs::create_dir_all(&destination_dir).await?; + + download_file_and_verify( + &self.log, + &tarball_path, + &tarball_url, + ChecksumAlgorithm::Sha2, + &checksum, + ) + .await?; + + unpack_tarball(&self.log, &tarball_path, &destination_dir).await?; + + Ok(()) + } + async fn download_clickhouse(&self) -> Result<()> { let os = os_name()?; diff --git a/tools/cargo_hack_checksum b/tools/cargo_hack_checksum new file mode 100644 index 00000000000..12ed33c12ef --- /dev/null +++ b/tools/cargo_hack_checksum @@ -0,0 +1,3 @@ +CIDL_SHA256_DARWIN="ee00750378126c7e14402a45c34f95ed1ba4be2ae505b0c0020bb39b5b3467a4" +CIDL_SHA256_ILLUMOS="f80d281343368bf7a027e2a7e94ae98a19e085c0666bff8d15264f39b42997bc" +CIDL_SHA256_LINUX="ffecd932fc7569975eb77d70f2e299f07b57220868bedeb5867062a4a95a0376" diff --git a/tools/cargo_hack_version b/tools/cargo_hack_version new file mode 100644 index 00000000000..cb180fda596 --- /dev/null +++ b/tools/cargo_hack_version @@ -0,0 +1 @@ +0.6.29 From b7886408a14a12455457e6a9922c0cb3cd5f60b1 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Mon, 15 Jul 2024 13:11:23 +0000 Subject: [PATCH 4/4] review: workspace path --- README.adoc | 7 ------- dev-tools/xtask/src/check_features.rs | 20 ++++++++++++-------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/README.adoc b/README.adoc index 2183f6e0faa..4979411d73d 100644 --- a/README.adoc +++ b/README.adoc @@ -120,13 +120,6 @@ This `xtask` is run in CI using the `--ci` parameter , which automatically exlud If `cargo hack` is not already installed in omicron's `out/` directory, a pre-built binary will be installed automatically depending on your operating system and architecture. -You can also run the the `xtask` with the `install-version ` option, which will install the cargo subcommand into your user's `.cargo` directory: - -[source,text] ----- -$ cargo xtask check-features --install-version 0.6.28 ----- - To limit the max number of simultaneous feature flags combined for checking, run the `xtask` with the `--depth ` flag: [source,text] diff --git a/dev-tools/xtask/src/check_features.rs b/dev-tools/xtask/src/check_features.rs index f4efd6791ce..a9dbc2bff7b 100644 --- a/dev-tools/xtask/src/check_features.rs +++ b/dev-tools/xtask/src/check_features.rs @@ -5,7 +5,7 @@ //! Subcommand: cargo xtask check-features use anyhow::{bail, Result}; -use camino::{Utf8Path, Utf8PathBuf}; +use camino::Utf8PathBuf; use clap::Parser; use std::{collections::HashSet, process::Command}; @@ -115,14 +115,18 @@ fn os_name() -> Result { Ok(os) } -/// Get the path to the `out` directory. +/// This is a workaround for the lack of a CARGO_WORKSPACE_DIR environment +/// variable, as suggested in . +/// A better workaround might be to set this in the `[env]` section of +/// `.cargo/config.toml`. +fn project_root() -> Utf8PathBuf { + Utf8PathBuf::from(&concat!(env!("CARGO_MANIFEST_DIR"), "/..")) +} + +/// Get the path to the `out` directory from the project root/workspace +/// directory. fn out_dir() -> Utf8PathBuf { - if let Ok(omicron_dir) = std::env::var("OMICRON") { - Utf8Path::new(format!("{}/out/cargo-hack", omicron_dir).as_str()) - .to_path_buf() - } else { - Utf8Path::new("out/cargo-hack").to_path_buf() - } + project_root().join("out/cargo-hack") } /// Install `cargo-hack` if the `install-version` was specified; otherwise,