xtask: Standardize command logging

Don't run `cargo build --verbose`; it's too noisy.

test/integration-test/tests/btf_relocations.rs

@@ -1,4 +1,4 @@
-use anyhow::{Context, Result};
+use anyhow::{bail, Context as _, Result};
 use std::{path::PathBuf, process::Command, thread::sleep, time::Duration};
 use tempfile::TempDir;
 
@@ -266,15 +266,20 @@ impl RelocationTest {
             "#
         ))
         .context("Failed to compile BTF")?;
-        Command::new("llvm-objcopy")
-            .current_dir(tmp_dir.path())
+        let mut cmd = Command::new("llvm-objcopy");
+        cmd.current_dir(tmp_dir.path())
             .args(["--dump-section", ".BTF=target.btf"])
-            .arg(compiled_file)
+            .arg(compiled_file);
+        let status = cmd
             .status()
-            .context("Failed to run llvm-objcopy")?
-            .success()
-            .then_some(())
-            .context("Failed to extract BTF")?;
+            .with_context(|| format!("Failed to run {cmd:?}"))?;
+        match status.code() {
+            Some(code) => match code {
+                0 => {}
+                code => bail!("{cmd:?} exited with code {code}"),
+            },
+            None => bail!("{cmd:?} terminated by signal"),
+        }
         let btf = Btf::parse_file(tmp_dir.path().join("target.btf"), Endianness::default())
             .context("Error parsing generated BTF")?;
         Ok(btf)
@@ -287,15 +292,20 @@ fn compile(source_code: &str) -> Result<(TempDir, PathBuf)> {
     let tmp_dir = tempfile::tempdir().context("Error making temp dir")?;
     let source = tmp_dir.path().join("source.c");
     std::fs::write(&source, source_code).context("Writing bpf program failed")?;
-    Command::new("clang")
-        .current_dir(&tmp_dir)
+    let mut cmd = Command::new("clang");
+    cmd.current_dir(&tmp_dir)
         .args(["-c", "-g", "-O2", "-target", "bpf"])
-        .arg(&source)
+        .arg(&source);
+    let status = cmd
         .status()
-        .context("Failed to run clang")?
-        .success()
-        .then_some(())
-        .context("Failed to compile eBPF source")?;
+        .with_context(|| format!("Failed to run {cmd:?}"))?;
+    match status.code() {
+        Some(code) => match code {
+            0 => {}
+            code => bail!("{cmd:?} exited with code {code}"),
+        },
+        None => bail!("{cmd:?} terminated by signal"),
+    }
     Ok((tmp_dir, source.with_extension("o")))
 }
 

xtask/Cargo.toml

@@ -12,5 +12,4 @@ clap = { version = "4", features = ["derive"] }
 indoc = "2.0"
 proc-macro2 = "1"
 quote = "1"
-serde_json = "1"
 syn = "2"

xtask/src/build_ebpf.rs

@@ -4,13 +4,13 @@ use std::{
     ffi::{OsStr, OsString},
     fs,
     path::{Path, PathBuf},
-    process::{Command, Output},
+    process::Command,
 };
 
-use anyhow::{bail, Context};
+use anyhow::Result;
 use clap::Parser;
 
-use crate::utils::workspace_root;
+use crate::utils::{exec, workspace_root};
 
 #[derive(Debug, Copy, Clone)]
 pub enum Architecture {
@@ -49,61 +49,56 @@ pub struct BuildEbpfOptions {
     pub libbpf_dir: PathBuf,
 }
 
-pub fn build_ebpf(opts: BuildEbpfOptions) -> anyhow::Result<()> {
+pub fn build_ebpf(opts: BuildEbpfOptions) -> Result<()> {
     build_rust_ebpf(&opts)?;
     build_c_ebpf(&opts)
 }
 
-fn build_rust_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> {
+fn build_rust_ebpf(opts: &BuildEbpfOptions) -> Result<()> {
+    let BuildEbpfOptions {
+        target,
+        libbpf_dir: _,
+    } = opts;
+
     let mut dir = PathBuf::from(workspace_root());
     dir.push("test/integration-ebpf");
 
-    let target = format!("--target={}", opts.target);
-    let args = vec![
-        "+nightly",
-        "build",
-        "--release",
-        "--verbose",
-        target.as_str(),
-        "-Z",
-        "build-std=core",
-    ];
-    let status = Command::new("cargo")
+    exec(
+        Command::new("cargo")
             .current_dir(&dir)
-        .args(&args)
-        .status()
-        .expect("failed to build bpf program");
-    assert!(status.success());
-    Ok(())
+            .args(["+nightly", "build", "--release", "--target"])
+            .arg(target.to_string())
+            .args(["-Z", "build-std=core"])
+            .current_dir(&dir),
+    )
 }
 
-fn get_libbpf_headers<P: AsRef<Path>>(libbpf_dir: P, include_path: P) -> anyhow::Result<()> {
-    let dir = include_path.as_ref();
-    fs::create_dir_all(dir)?;
+fn get_libbpf_headers(libbpf_dir: &Path, include_path: &Path) -> Result<()> {
+    fs::create_dir_all(include_path)?;
     let mut includedir = OsString::new();
     includedir.push("INCLUDEDIR=");
-    includedir.push(dir.as_os_str());
-    let status = Command::new("make")
-        .current_dir(libbpf_dir.as_ref().join("src"))
+    includedir.push(include_path);
+    exec(
+        Command::new("make")
+            .current_dir(libbpf_dir.join("src"))
             .arg(includedir)
-        .arg("install_headers")
-        .status()
-        .expect("failed to build get libbpf headers");
-    assert!(status.success());
-    Ok(())
+            .arg("install_headers"),
+    )
 }
 
-fn build_c_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> {
+fn build_c_ebpf(opts: &BuildEbpfOptions) -> Result<()> {
+    let BuildEbpfOptions { target, libbpf_dir } = opts;
+
     let mut src = PathBuf::from(workspace_root());
     src.push("test/integration-ebpf/src/bpf");
 
     let mut out_path = PathBuf::from(workspace_root());
     out_path.push("target");
-    out_path.push(opts.target.to_string());
+    out_path.push(target.to_string());
     out_path.push("release");
 
     let include_path = out_path.join("include");
-    get_libbpf_headers(&opts.libbpf_dir, &include_path)?;
+    get_libbpf_headers(libbpf_dir, &include_path)?;
     let files = fs::read_dir(&src).unwrap();
     for file in files {
         let p = file.unwrap().path();
@@ -120,11 +115,7 @@ fn build_c_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> {
 }
 
 /// Build eBPF programs with clang and libbpf headers.
-fn compile_with_clang<P: Clone + AsRef<Path>>(
-    src: P,
-    out: P,
-    include_path: P,
-) -> anyhow::Result<()> {
+fn compile_with_clang(src: &Path, out: &Path, include_path: &Path) -> Result<()> {
     let clang: Cow<'_, _> = match env::var_os("CLANG") {
         Some(val) => val.into(),
         None => OsStr::new("/usr/bin/clang").into(),
@@ -134,36 +125,14 @@ fn compile_with_clang<P: Clone + AsRef<Path>>(
         "aarch64" => "arm64",
         arch => arch,
     };
-    let mut cmd = Command::new(clang);
-    cmd.arg("-v")
+    exec(
+        Command::new(clang)
             .arg("-I")
-        .arg(include_path.as_ref())
-        .arg("-g")
-        .arg("-O2")
-        .arg("-target")
-        .arg("bpf")
-        .arg("-c")
+            .arg(include_path)
+            .args(["-g", "-O2", "-target", "bpf", "-c"])
             .arg(format!("-D__TARGET_ARCH_{arch}"))
-        .arg(src.as_ref().as_os_str())
+            .arg(src)
             .arg("-o")
-        .arg(out.as_ref().as_os_str());
-
-    let Output {
-        status,
-        stdout,
-        stderr,
-    } = cmd.output().context("Failed to execute clang")?;
-    if !status.success() {
-        bail!(
-            "Failed to compile eBPF programs\n \
-            stdout=\n \
-            {}\n \
-            stderr=\n \
-            {}\n",
-            String::from_utf8(stdout).unwrap(),
-            String::from_utf8(stderr).unwrap()
-        );
-    }
-
-    Ok(())
+            .arg(out),
+    )
 }

xtask/src/build_test.rs

@@ -1,7 +1,8 @@
+use anyhow::Result;
 use clap::Parser;
 use std::process::Command;
 
-use crate::build_ebpf;
+use crate::{build_ebpf, utils::exec};
 
 #[derive(Parser)]
 pub struct Options {
@@ -13,20 +14,19 @@ pub struct Options {
     pub ebpf_options: build_ebpf::BuildEbpfOptions,
 }
 
-pub fn build_test(opts: Options) -> anyhow::Result<()> {
-    build_ebpf::build_ebpf(opts.ebpf_options)?;
+pub fn build_test(opts: Options) -> Result<()> {
+    let Options {
+        musl_target,
+        ebpf_options,
+    } = opts;
 
-    let mut args = ["build", "-p", "integration-test", "--verbose"]
-        .iter()
-        .map(|s| s.to_string())
-        .collect::<Vec<_>>();
-    if let Some(target) = opts.musl_target {
-        args.push(format!("--target={target}"));
+    build_ebpf::build_ebpf(ebpf_options)?;
+
+    let mut cmd = Command::new("cargo");
+    cmd.args(["build", "-p", "integration-test"]);
+
+    if let Some(target) = musl_target {
+        cmd.args(["--target", &target]);
     }
-    let status = Command::new("cargo")
-        .args(&args)
-        .status()
-        .expect("failed to build bpf program");
-    assert!(status.success());
-    Ok(())
+    exec(&mut cmd)
 }

xtask/src/docs/mod.rs

@@ -1,3 +1,5 @@
+use crate::utils::exec;
+use anyhow::{Context as _, Result};
 use std::{
     path::{Path, PathBuf},
     process::Command,
@@ -7,7 +9,7 @@ use std::{fs, io, io::Write};
 
 use indoc::indoc;
 
-pub fn docs() -> Result<(), anyhow::Error> {
+pub fn docs() -> Result<()> {
     let current_dir = PathBuf::from(".");
     let header_path = current_dir.join("header.html");
     let mut header = fs::File::create(&header_path).expect("can't create header.html");
@@ -19,8 +21,11 @@ pub fn docs() -> Result<(), anyhow::Error> {
 
     build_docs(&current_dir.join("aya"), &abs_header_path)?;
     build_docs(&current_dir.join("bpf/aya-bpf"), &abs_header_path)?;
-    copy_dir_all("./target/doc", "./site/user")?;
-    copy_dir_all("./target/bpfel-unknown-none/doc", "./site/bpf")?;
+    copy_dir_all("./target/doc".as_ref(), "./site/user".as_ref())?;
+    copy_dir_all(
+        "./target/bpfel-unknown-none/doc".as_ref(),
+        "./site/bpf".as_ref(),
+    )?;
 
     let mut robots = fs::File::create("site/robots.txt").expect("can't create robots.txt");
     robots
@@ -53,17 +58,15 @@ pub fn docs() -> Result<(), anyhow::Error> {
     Ok(())
 }
 
-fn build_docs(working_dir: &PathBuf, abs_header_path: &Path) -> Result<(), anyhow::Error> {
-    let replace = Command::new("sed")
-        .current_dir(working_dir)
-        .args(vec!["-i.bak", "s/crabby.svg/crabby_dev.svg/", "src/lib.rs"])
-        .status()
-        .expect("failed to replace logo");
-    assert!(replace.success());
-
-    let args = vec!["+nightly", "doc", "--no-deps", "--all-features"];
+fn build_docs(working_dir: &Path, abs_header_path: &Path) -> Result<()> {
+    exec(Command::new("sed").current_dir(working_dir).args([
+        "-i.bak",
+        "s/crabby.svg/crabby_dev.svg/",
+        "src/lib.rs",
+    ]))?;
 
-    let status = Command::new("cargo")
+    exec(
+        Command::new("cargo")
             .current_dir(working_dir)
             .env(
                 "RUSTDOCFLAGS",
@@ -72,30 +75,29 @@ fn build_docs(working_dir: &PathBuf, abs_header_path: &Path) -> Result<(), anyho
                     abs_header_path.to_str().unwrap()
                 ),
             )
-        .args(args)
-        .status()
-        .expect("failed to build aya docs");
-    assert!(status.success());
+            .args(["+nightly", "doc", "--no-deps", "--all-features"]),
+    )?;
+
     fs::rename(
         working_dir.join("src/lib.rs.bak"),
         working_dir.join("src/lib.rs"),
     )
-    .unwrap();
-    Ok(())
+    .context("Failed to rename lib.rs.bak to lib.rs")
 }
 
-fn copy_dir_all<P1: AsRef<Path>, P2: AsRef<Path>>(src: P1, dst: P2) -> io::Result<()> {
-    fs::create_dir_all(&dst)?;
+fn copy_dir_all(src: &Path, dst: &Path) -> io::Result<()> {
+    fs::create_dir_all(dst)?;
     for entry in fs::read_dir(src)? {
         let entry = entry?;
         let ty = entry.file_type()?;
+        let src = entry.path();
+        let src = src.as_path();
+        let dst = dst.join(entry.file_name());
+        let dst = dst.as_path();
         if ty.is_dir() {
-            copy_dir_all(entry.path(), dst.as_ref().join(entry.file_name()))?;
-        } else {
-            let new_path = dst.as_ref().join(entry.file_name());
-            if !new_path.exists() {
-                fs::copy(entry.path(), dst.as_ref().join(entry.file_name()))?;
-            }
+            copy_dir_all(src, dst)?;
+        } else if !dst.exists() {
+            fs::copy(src, dst)?;
         }
     }
     Ok(())

xtask/src/run.rs

@@ -5,7 +5,7 @@ use std::{
     process::{Command, Stdio},
 };
 
-use anyhow::Context as _;
+use anyhow::{Context as _, Result};
 use cargo_metadata::{Artifact, CompilerMessage, Message, Target};
 use clap::Parser;
 
@@ -31,12 +31,14 @@ pub struct Options {
 }
 
 /// Build the project
-fn build(release: bool) -> Result<Vec<(PathBuf, PathBuf)>, anyhow::Error> {
+fn build(release: bool) -> Result<Vec<(PathBuf, PathBuf)>> {
     let mut cmd = Command::new("cargo");
-    cmd.arg("build")
-        .arg("--tests")
-        .arg("--message-format=json")
-        .arg("--package=integration-test");
+    cmd.args([
+        "build",
+        "--tests",
+        "--message-format=json",
+        "--package=integration-test",
+    ]);
     if release {
         cmd.arg("--release");
     }
@@ -83,7 +85,7 @@ fn build(release: bool) -> Result<Vec<(PathBuf, PathBuf)>, anyhow::Error> {
 }
 
 /// Build and run the project
-pub fn run(opts: Options) -> Result<(), anyhow::Error> {
+pub fn run(opts: Options) -> Result<()> {
     let Options {
         bpf_target,
         release,
@@ -116,10 +118,8 @@ pub fn run(opts: Options) -> Result<(), anyhow::Error> {
         println!("{} running {cmd:?}", src_path.display());
 
         let status = cmd
-            .stdout(Stdio::inherit())
-            .stderr(Stdio::inherit())
             .status()
-            .context("failed to run {cmd:?}")?;
+            .with_context(|| format!("failed to run {cmd:?}"))?;
         match status.code() {
             Some(code) => match code {
                 0 => {}

xtask/src/utils.rs

@@ -1,15 +1,24 @@
-use serde_json::Value;
 use std::{cell::OnceCell, process::Command};
 
+use anyhow::{bail, Context as _, Result};
+
 pub fn workspace_root() -> &'static str {
     static mut WORKSPACE_ROOT: OnceCell<String> = OnceCell::new();
     unsafe { &mut WORKSPACE_ROOT }.get_or_init(|| {
-        let output = Command::new("cargo").arg("metadata").output().unwrap();
-        if !output.status.success() {
-            panic!("unable to run cargo metadata")
-        }
-        let stdout = String::from_utf8(output.stdout).unwrap();
-        let v: Value = serde_json::from_str(&stdout).unwrap();
-        v["workspace_root"].as_str().unwrap().to_string()
+        let cmd = cargo_metadata::MetadataCommand::new();
+        cmd.exec().unwrap().workspace_root.to_string()
     })
 }
+
+pub fn exec(cmd: &mut Command) -> Result<()> {
+    let status = cmd
+        .status()
+        .with_context(|| format!("failed to run {cmd:?}"))?;
+    match status.code() {
+        Some(code) => match code {
+            0 => Ok(()),
+            code => bail!("{cmd:?} exited with code {code}"),
+        },
+        None => bail!("{cmd:?} terminated by signal"),
+    }
+}