From b0a4ab5f20b9838d035913e48fc350b97e8e4a06 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sun, 9 Jul 2023 16:05:06 -0400 Subject: [PATCH] xtask: Standardize command logging Don't run `cargo build --verbose`; it's too noisy. --- .../integration-test/tests/btf_relocations.rs | 40 +++--- xtask/Cargo.toml | 1 - xtask/src/build_ebpf.rs | 117 +++++++----------- xtask/src/build_test.rs | 30 ++--- xtask/src/docs/mod.rs | 70 ++++++----- xtask/src/run.rs | 20 +-- xtask/src/utils.rs | 25 ++-- 7 files changed, 146 insertions(+), 157 deletions(-) diff --git a/test/integration-test/tests/btf_relocations.rs b/test/integration-test/tests/btf_relocations.rs index 2811f74a..34bc3dbe 100644 --- a/test/integration-test/tests/btf_relocations.rs +++ b/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"))) } diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 2f8a74d5..24d91ef4 100644 --- a/xtask/Cargo.toml +++ b/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" diff --git a/xtask/src/build_ebpf.rs b/xtask/src/build_ebpf.rs index 5a9889c3..bdbf44ed 100644 --- a/xtask/src/build_ebpf.rs +++ b/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") - .current_dir(&dir) - .args(&args) - .status() - .expect("failed to build bpf program"); - assert!(status.success()); - Ok(()) + exec( + Command::new("cargo") + .current_dir(&dir) + .args(["+nightly", "build", "--release", "--target"]) + .arg(target.to_string()) + .args(["-Z", "build-std=core"]) + .current_dir(&dir), + ) } -fn get_libbpf_headers>(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")) - .arg(includedir) - .arg("install_headers") - .status() - .expect("failed to build get libbpf headers"); - assert!(status.success()); - Ok(()) + includedir.push(include_path); + exec( + Command::new("make") + .current_dir(libbpf_dir.join("src")) + .arg(includedir) + .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>( - 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>( "aarch64" => "arm64", arch => arch, }; - let mut cmd = Command::new(clang); - cmd.arg("-v") - .arg("-I") - .arg(include_path.as_ref()) - .arg("-g") - .arg("-O2") - .arg("-target") - .arg("bpf") - .arg("-c") - .arg(format!("-D__TARGET_ARCH_{arch}")) - .arg(src.as_ref().as_os_str()) - .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(()) + exec( + Command::new(clang) + .arg("-I") + .arg(include_path) + .args(["-g", "-O2", "-target", "bpf", "-c"]) + .arg(format!("-D__TARGET_ARCH_{arch}")) + .arg(src) + .arg("-o") + .arg(out), + ) } diff --git a/xtask/src/build_test.rs b/xtask/src/build_test.rs index b45943a1..0fe4c277 100644 --- a/xtask/src/build_test.rs +++ b/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::>(); - 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) } diff --git a/xtask/src/docs/mod.rs b/xtask/src/docs/mod.rs index bfeca3a9..d392798c 100644 --- a/xtask/src/docs/mod.rs +++ b/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(¤t_dir.join("aya"), &abs_header_path)?; build_docs(¤t_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,49 +58,46 @@ 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()); +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 args = vec!["+nightly", "doc", "--no-deps", "--all-features"]; + exec( + Command::new("cargo") + .current_dir(working_dir) + .env( + "RUSTDOCFLAGS", + format!( + "--cfg docsrs --html-in-header {} -D warnings", + abs_header_path.to_str().unwrap() + ), + ) + .args(["+nightly", "doc", "--no-deps", "--all-features"]), + )?; - let status = Command::new("cargo") - .current_dir(working_dir) - .env( - "RUSTDOCFLAGS", - format!( - "--cfg docsrs --html-in-header {} -D warnings", - abs_header_path.to_str().unwrap() - ), - ) - .args(args) - .status() - .expect("failed to build aya docs"); - assert!(status.success()); 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, P2: AsRef>(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(()) diff --git a/xtask/src/run.rs b/xtask/src/run.rs index 42ec51b9..0c7a241d 100644 --- a/xtask/src/run.rs +++ b/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, anyhow::Error> { +fn build(release: bool) -> Result> { 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, 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 => {} diff --git a/xtask/src/utils.rs b/xtask/src/utils.rs index e1e3eb2a..f2cd2471 100644 --- a/xtask/src/utils.rs +++ b/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 = 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"), + } +}