From 45be74a6e0fdd1b5f5bd5b13eab71ee83fbbad26 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Sat, 13 Jul 2024 01:58:23 +0000 Subject: [PATCH] work with pre-built bins --- .github/buildomat/jobs/check-features.sh | 10 +- .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(+), 56 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..0c6259e61ba 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. @@ -23,14 +25,14 @@ 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