Skip to content

Commit

Permalink
move the verify-libraries CI check into releng (#5779)
Browse files Browse the repository at this point in the history
In the context of the build-and-test job, `cargo xtask verify-libraries`
is a bit of an unrelated operation, because it's building all of the
binaries in between building all of the test binaries and then running
all of the test binaries.

I had initially considered moving it to its own Buildomat job, but then
realized that this is really a release engineering check; we already
have built most of the binaries (in particular, Nexus, the one that
takes the longest to build) in order to package them up, so we can
manage to build the handful of extra utility binaries and perform the
checks while the host OS image builds.

This should improve the **build-and-test (helios)** job by about 10
minutes while having negligible impact on the **tuf-repo** job, and will
also eliminate a new and exciting source of test flake we've seen where
`timeout 10m` is getting hit, presumably because we continue to write
software.
  • Loading branch information
iliana committed May 15, 2024
1 parent 9ddc97c commit e0a1184
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 50 deletions.
13 changes: 0 additions & 13 deletions .github/buildomat/build-and-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,6 @@ export RUSTC_BOOTSTRAP=1
# We report build progress to stderr, and the "--timings=json" output goes to stdout.
ptime -m cargo build -Z unstable-options --timings=json --workspace --tests --locked --verbose 1> "$OUTPUT_DIR/crate-build-timings.json"

# If we are running on illumos we want to verify that we are not requiring
# system libraries outside of specific binaries. If we encounter this situation
# we bail.
# NB: `cargo xtask verify-libraries` runs `cargo build --bins` to ensure it can
# check the final executables.
if [[ $target_os == "illumos" ]]; then
banner verify-libraries
# This has a separate timeout from `cargo nextest` since `timeout` expects
# to run an external command and therefore we cannot run bash functions or
# subshells.
ptime -m timeout 10m cargo xtask verify-libraries
fi

#
# We apply our own timeout to ensure that we get a normal failure on timeout
# rather than a buildomat timeout. See oxidecomputer/buildomat#8.
Expand Down
1 change: 0 additions & 1 deletion .github/buildomat/jobs/a4x2-deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#: name = "a4x2-deploy"
#: variety = "basic"
#: target = "lab-2.0-opte-0.27"
#: rust_toolchain = "stable"
#: output_rules = [
#: "/out/falcon/*.log",
#: "/out/falcon/*.err",
Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/jobs/a4x2-prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#: name = "a4x2-prepare"
#: variety = "basic"
#: target = "helios-2.0"
#: rust_toolchain = "stable"
#: rust_toolchain = "1.77.2"
#: output_rules = [
#: "=/out/cargo-bay-ce.tgz",
#: "=/out/cargo-bay-cr1.tgz",
Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/jobs/build-and-test-helios.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#: name = "build-and-test (helios)"
#: variety = "basic"
#: target = "helios-2.0"
#: rust_toolchain = "1.72.1"
#: rust_toolchain = "1.77.2"
#: output_rules = [
#: "%/work/*",
#: "%/var/tmp/omicron_tmp/*",
Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/jobs/build-and-test-linux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#: name = "build-and-test (ubuntu-22.04)"
#: variety = "basic"
#: target = "ubuntu-22.04"
#: rust_toolchain = "1.72.1"
#: rust_toolchain = "1.77.2"
#: output_rules = [
#: "%/work/*",
#: "%/var/tmp/omicron_tmp/*",
Expand Down
2 changes: 1 addition & 1 deletion .github/buildomat/jobs/clippy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#: name = "clippy (helios)"
#: variety = "basic"
#: target = "helios-2.0"
#: rust_toolchain = "1.72.1"
#: rust_toolchain = "1.77.2"
#: output_rules = []

# Run clippy on illumos (not just other systems) because a bunch of our code
Expand Down
13 changes: 12 additions & 1 deletion dev-tools/releng/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,18 @@ async fn main() -> Result<()> {
jobs.select("host-image").after("host-profile");

stamp_packages!("tuf-stamp", Target::Host, TUF_PACKAGES)
.after("host-stamp");
.after("host-stamp")
.after("recovery-stamp");

// Run `cargo xtask verify-libraries --release`. (This was formerly run in
// the build-and-test Buildomat job, but this fits better here where we've
// already built most of the binaries.)
jobs.push_command(
"verify-libraries",
Command::new(&cargo).args(["xtask", "verify-libraries", "--release"]),
)
.after("host-package")
.after("recovery-package");

for (name, base_url) in [
("staging", "https://permslip-staging.corp.oxide.computer"),
Expand Down
34 changes: 21 additions & 13 deletions dev-tools/xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@ use clap::{Parser, Subcommand};

mod check_workspace_deps;
mod clippy;

#[cfg(target_os = "illumos")]
mod verify_libraries;

#[cfg(target_os = "illumos")]
mod virtual_hardware;
#[cfg(not(target_os = "illumos"))]
#[path = "virtual_hardware_stub.rs"]
mod virtual_hardware;

#[derive(Parser)]
#[command(name = "cargo xtask", about = "Workspace-related developer tools")]
Expand All @@ -35,27 +32,38 @@ enum Cmds {
CheckWorkspaceDeps,
/// Run configured clippy checks
Clippy(clippy::ClippyArgs),

/// Verify we are not leaking library bindings outside of intended
/// crates
VerifyLibraries,
#[cfg(target_os = "illumos")]
VerifyLibraries(verify_libraries::Args),
/// Manage virtual hardware
#[cfg(target_os = "illumos")]
VirtualHardware(virtual_hardware::Args),

/// (this command is only available on illumos)
#[cfg(not(target_os = "illumos"))]
VerifyLibraries,
/// (this command is only available on illumos)
#[cfg(not(target_os = "illumos"))]
VirtualHardware,
}

fn main() -> Result<()> {
let args = Args::parse();
match args.cmd {
Cmds::Clippy(args) => clippy::run_cmd(args),
Cmds::CheckWorkspaceDeps => check_workspace_deps::run_cmd(),
Cmds::VerifyLibraries => {
#[cfg(target_os = "illumos")]
return verify_libraries::run_cmd();
#[cfg(not(target_os = "illumos"))]
unimplemented!(
"Library verification is only available on illumos!"
);
}

#[cfg(target_os = "illumos")]
Cmds::VerifyLibraries(args) => verify_libraries::run_cmd(args),
#[cfg(target_os = "illumos")]
Cmds::VirtualHardware(args) => virtual_hardware::run_cmd(args),

#[cfg(not(target_os = "illumos"))]
Cmds::VerifyLibraries | Cmds::VirtualHardware => {
anyhow::bail!("this command is only available on illumos");
}
}
}

Expand Down
27 changes: 22 additions & 5 deletions dev-tools/xtask/src/verify_libraries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use anyhow::{bail, Context, Result};
use camino::Utf8Path;
use cargo_metadata::Message;
use clap::Parser;
use fs_err as fs;
use serde::Deserialize;
use std::{
Expand All @@ -16,6 +17,13 @@ use swrite::{swriteln, SWrite};

use crate::load_workspace;

#[derive(Parser)]
pub struct Args {
/// Build in release mode
#[clap(long)]
release: bool,
}

#[derive(Deserialize, Debug)]
struct LibraryConfig {
binary_allow_list: Option<BTreeSet<String>>,
Expand Down Expand Up @@ -83,20 +91,29 @@ fn verify_executable(

Ok(())
}
pub fn run_cmd() -> Result<()> {

pub fn run_cmd(args: Args) -> Result<()> {
let metadata = load_workspace()?;
let mut config_path = metadata.workspace_root;
config_path.push(".cargo/xtask.toml");
let config = read_xtask_toml(&config_path)?;

let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());
let mut command = Command::new(cargo)
.args(["build", "--bins", "--message-format=json-render-diagnostics"])
let mut command = Command::new(cargo);
command.args([
"build",
"--bins",
"--message-format=json-render-diagnostics",
]);
if args.release {
command.arg("--release");
}
let mut child = command
.stdout(Stdio::piped())
.spawn()
.context("failed to spawn cargo build")?;

let reader = BufReader::new(command.stdout.take().context("take stdout")?);
let reader = BufReader::new(child.stdout.take().context("take stdout")?);

let mut errors = Default::default();
for message in cargo_metadata::Message::parse_stream(reader) {
Expand All @@ -108,7 +125,7 @@ pub fn run_cmd() -> Result<()> {
}
}

let status = command.wait()?;
let status = child.wait()?;
if !status.success() {
bail!("Failed to execute cargo build successfully {}", status);
}
Expand Down
13 changes: 0 additions & 13 deletions dev-tools/xtask/src/virtual_hardware_stub.rs

This file was deleted.

0 comments on commit e0a1184

Please sign in to comment.