From 5811d6ff5687ae115cc8876ddf511dd01c02539b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 10 Oct 2024 16:07:26 -0400 Subject: [PATCH] Replace xtask builds with build scripts Adapt https://github.com/aya-rs/aya/commit/3d463a3 and subsequent work to the template. This has worked very well for us in the main project, and our users should get the same hotness. Note that xtask is still used for running, as it is in the main project. --- Cargo.toml | 13 +-- README.md | 29 +----- test.sh | 9 +- xtask/Cargo.toml | 2 +- xtask/src/build.rs | 41 -------- xtask/src/build_ebpf.rs | 68 ------------ xtask/src/lib.rs | 1 + xtask/src/main.rs | 23 +---- xtask/src/run.rs | 76 ++++++-------- {{project-name}}-ebpf/Cargo.toml | 4 + {{project-name}}-ebpf/build.rs | 30 ++++++ {{project-name}}-ebpf/src/lib.rs | 3 + {{project-name}}/Cargo.toml | 17 +++ {{project-name}}/build.rs | 172 +++++++++++++++++++++++++++++++ {{project-name}}/src/main.rs | 11 +- 15 files changed, 283 insertions(+), 216 deletions(-) delete mode 100644 xtask/src/build.rs delete mode 100644 xtask/src/build_ebpf.rs create mode 100644 xtask/src/lib.rs create mode 100644 {{project-name}}-ebpf/build.rs create mode 100644 {{project-name}}-ebpf/src/lib.rs create mode 100644 {{project-name}}/build.rs diff --git a/Cargo.toml b/Cargo.toml index b411146..a833d12 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ aya-log = { version = "0.2.1", default-features = false } aya-log-ebpf = { version = "0.1.1", default-features = false } anyhow = { version = "1", default-features = false } +cargo_metadata = { version = "0.18.0", default-features = false } # `std` feature is currently required to build `clap`. # # See https://github.com/clap-rs/clap/blob/61f5ee5/clap_builder/src/lib.rs#L15. @@ -18,18 +19,14 @@ env_logger = { version = "0.11.5", default-features = false } libc = { version = "0.2.159", default-features = false } log = { version = "0.4.22", default-features = false } tokio = { version = "1.40.0", default-features = false } +which = { version = "6.0.0", default-features = false } [profile.dev] -opt-level = 3 -debug = false -overflow-checks = false -lto = true panic = "abort" -incremental = false -codegen-units = 1 -rpath = false [profile.release] -lto = true panic = "abort" + +[profile.release.package.{{project-name}}-ebpf] +debug = 2 codegen-units = 1 diff --git a/README.md b/README.md index bee17f6..c870678 100644 --- a/README.md +++ b/README.md @@ -4,29 +4,10 @@ 1. Install bpf-linker: `cargo install bpf-linker` -## Build eBPF +## Build & Run -```bash -cargo xtask build-ebpf -``` +Use `cargo build`, `cargo check`, etc. as normal. Run your program with `xtask run`. -To perform a release build you can use the `--release` flag. -You may also change the target architecture with the `--target` flag. - -## Build Userspace - -```bash -cargo build -``` - -## Build eBPF and Userspace - -```bash -cargo xtask build -``` - -## Run - -```bash -RUST_LOG=info cargo xtask run -``` +Cargo build scripts are used to automatically build the eBPF correctly and include it in the +program. When not using `xtask run`, eBPF code generation is skipped for a faster developer +experience; this compromise necessitates the use of `xtask` to actually build the eBPF. diff --git a/test.sh b/test.sh index 87a7e6a..9e0354c 100755 --- a/test.sh +++ b/test.sh @@ -51,12 +51,11 @@ esac cargo generate --path "${TEMPLATE_DIR}" -n test -d program_type="${PROG_TYPE}" ${ADDITIONAL_ARGS} pushd test -cargo xtask build -cargo xtask build --release +cargo build --package test +cargo build --package test --release # We cannot run clippy over the whole workspace at once due to feature unification. Since both test -# and test-ebpf both depend on test-common and test activates test-common's aya dependency, we end -# up trying to compile the panic handler twice: once from the bpf program, and again from std via -# aya. +# and test-ebpf depend on test-common and test activates test-common's aya dependency, we end up +# trying to compile the panic handler twice: once from the bpf program, and again from std via aya. cargo clippy --exclude test-ebpf --all-targets --workspace -- --deny warnings cargo clippy --package test-ebpf --all-targets -- --deny warnings popd diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index ed6bada..6f639a8 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -4,5 +4,5 @@ version = "0.1.0" edition = "2021" [dependencies] -anyhow = { workspace = true } +anyhow = { workspace = true, default-features = true } clap = { workspace = true, default-features = true, features = ["derive"] } diff --git a/xtask/src/build.rs b/xtask/src/build.rs deleted file mode 100644 index 04709d9..0000000 --- a/xtask/src/build.rs +++ /dev/null @@ -1,41 +0,0 @@ -use std::process::Command; - -use anyhow::Context as _; -use clap::Parser; - -use crate::build_ebpf::{build_ebpf, Architecture, Options as BuildOptions}; - -#[derive(Debug, Parser)] -pub struct Options { - /// Set the endianness of the BPF target - #[clap(default_value = "bpfel-unknown-none", long)] - pub bpf_target: Architecture, - /// Build and run the release target - #[clap(long)] - pub release: bool, -} - -/// Build our ebpf program and the userspace program. -pub fn build(opts: Options) -> Result<(), anyhow::Error> { - let Options { - bpf_target, - release, - } = opts; - - // Build our ebpf program. - build_ebpf(BuildOptions { - target: bpf_target, - release, - })?; - - // Build our userspace program. - let mut cmd = Command::new("cargo"); - cmd.arg("build"); - if release { - cmd.arg("--release"); - } - let status = cmd.status().context("failed to build userspace")?; - anyhow::ensure!(status.success(), "failed to build userspace program: {}", status); - - Ok(()) -} diff --git a/xtask/src/build_ebpf.rs b/xtask/src/build_ebpf.rs deleted file mode 100644 index ca83f1d..0000000 --- a/xtask/src/build_ebpf.rs +++ /dev/null @@ -1,68 +0,0 @@ -use std::process::Command; - -use anyhow::Context as _; -use clap::Parser; - -#[derive(Debug, Clone)] -pub enum Architecture { - BpfEl, - BpfEb, -} - -impl Architecture { - pub fn as_str(&self) -> &'static str { - match self { - Architecture::BpfEl => "bpfel-unknown-none", - Architecture::BpfEb => "bpfeb-unknown-none", - } - } -} - -impl std::str::FromStr for Architecture { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - Ok(match s { - "bpfel-unknown-none" => Architecture::BpfEl, - "bpfeb-unknown-none" => Architecture::BpfEb, - _ => return Err("invalid target"), - }) - } -} - -impl std::fmt::Display for Architecture { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.as_str()) - } -} - -#[derive(Debug, Parser)] -pub struct Options { - /// Set the endianness of the BPF target - #[clap(default_value = "bpfel-unknown-none", long)] - pub target: Architecture, - /// Build the release target - #[clap(long)] - pub release: bool, -} - -pub fn build_ebpf(opts: Options) -> Result<(), anyhow::Error> { - let Options { target, release } = opts; - - let mut cmd = Command::new("cargo"); - cmd.current_dir("{{project-name}}-ebpf") - // Command::new creates a child process which inherits all env variables. This means env - // vars set by the cargo xtask command are also inherited. RUSTUP_TOOLCHAIN is removed so - // the rust-toolchain.toml file in the -ebpf folder is honored. - .env_remove("RUSTUP_TOOLCHAIN") - .args(["build", "--target", target.as_str()]); - - if release { - cmd.arg("--release"); - } - - let status = cmd.status().context("failed to build bpf program")?; - anyhow::ensure!(status.success(), "failed to build bpf program: {}", status); - - Ok(()) -} diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs new file mode 100644 index 0000000..fd8cbb0 --- /dev/null +++ b/xtask/src/lib.rs @@ -0,0 +1 @@ +pub const AYA_BUILD_EBPF: &str = "AYA_BUILD_EBPF"; diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 5079458..b59e543 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -1,9 +1,6 @@ -mod build_ebpf; -mod build; mod run; -use std::process::exit; - +use anyhow::Result; use clap::Parser; #[derive(Debug, Parser)] @@ -14,23 +11,13 @@ pub struct Options { #[derive(Debug, Parser)] enum Command { - BuildEbpf(build_ebpf::Options), - Build(build::Options), Run(run::Options), } -fn main() { - let opts = Options::parse(); - - use Command::*; - let ret = match opts.command { - BuildEbpf(opts) => build_ebpf::build_ebpf(opts), - Run(opts) => run::run(opts), - Build(opts) => build::build(opts), - }; +fn main() -> Result<()> { + let Options { command } = Parser::parse(); - if let Err(e) = ret { - eprintln!("{e:#}"); - exit(1); + match command { + Command::Run(opts) => run::run(opts), } } diff --git a/xtask/src/run.rs b/xtask/src/run.rs index d50bd11..16c80a8 100644 --- a/xtask/src/run.rs +++ b/xtask/src/run.rs @@ -1,55 +1,47 @@ -use std::process::Command; +use std::{ffi::OsString, process::Command}; -use anyhow::Context as _; +use anyhow::{bail, Context as _, Result}; use clap::Parser; - -use crate::{build::{build, Options as BuildOptions}, build_ebpf::Architecture}; +use xtask::AYA_BUILD_EBPF; #[derive(Debug, Parser)] pub struct Options { - /// Set the endianness of the BPF target - #[clap(default_value = "bpfel-unknown-none", long)] - pub bpf_target: Architecture, - /// Build and run the release target + /// Build and run the release target. #[clap(long)] - pub release: bool, - /// The command used to wrap your application + release: bool, + /// The command used to wrap your application. #[clap(short, long, default_value = "sudo -E")] - pub runner: String, - /// Arguments to pass to your application - #[clap(name = "args", last = true)] - pub run_args: Vec, + runner: String, + /// Arguments to pass to your application. + #[clap(global = true, last = true)] + run_args: Vec, } - -/// Build and run the project -pub fn run(opts: Options) -> Result<(), anyhow::Error> { - // Build our ebpf program and the project - build(BuildOptions{ - bpf_target: opts.bpf_target, - release: opts.release, - }).context("Error while building project")?; - - // profile we are building (release or debug) - let profile = if opts.release { "release" } else { "debug" }; - let bin_path = format!("target/{profile}/{{project-name}}"); - - // arguments to pass to the application - let mut run_args: Vec<_> = opts.run_args.iter().map(String::as_str).collect(); - - // configure args - let mut args: Vec<_> = opts.runner.trim().split_terminator(' ').collect(); - args.push(bin_path.as_str()); - args.append(&mut run_args); - - // run the command - let status = Command::new(args.first().expect("No first argument")) - .args(args.iter().skip(1)) +/// Build and run the project. +pub fn run(opts: Options) -> Result<()> { + let Options { + release, + runner, + run_args, + } = opts; + + let mut cmd = Command::new("cargo"); + cmd.env(AYA_BUILD_EBPF, "true"); + cmd.args(["run", "--package", "{{project-name}}", "--config"]); + if release { + cmd.arg(format!("target.\"cfg(all())\".runner=\"{}\"", runner)); + cmd.arg("--release"); + } else { + cmd.arg(format!("target.\"cfg(all())\".runner=\"{}\"", runner)); + } + if !run_args.is_empty() { + cmd.arg("--").args(run_args); + } + let status = cmd .status() - .expect("failed to run the command"); - - if !status.success() { - anyhow::bail!("Failed to run `{}`", args.join(" ")); + .with_context(|| format!("failed to run {cmd:?}"))?; + if status.code() != Some(0) { + bail!("{cmd:?} failed: {status:?}") } Ok(()) } diff --git a/{{project-name}}-ebpf/Cargo.toml b/{{project-name}}-ebpf/Cargo.toml index 4f5f274..578c50e 100644 --- a/{{project-name}}-ebpf/Cargo.toml +++ b/{{project-name}}-ebpf/Cargo.toml @@ -9,6 +9,10 @@ edition = "2021" aya-ebpf = { workspace = true } aya-log-ebpf = { workspace = true } +[build-dependencies] +which = { workspace = true } +xtask = { path = "../xtask" } + [[bin]] name = "{{ project-name }}" path = "src/main.rs" diff --git a/{{project-name}}-ebpf/build.rs b/{{project-name}}-ebpf/build.rs new file mode 100644 index 0000000..101ade2 --- /dev/null +++ b/{{project-name}}-ebpf/build.rs @@ -0,0 +1,30 @@ +use std::env; + +use which::which; +use xtask::AYA_BUILD_EBPF; + +/// Building this crate has an undeclared dependency on the `bpf-linker` binary. This would be +/// better expressed by [artifact-dependencies][bindeps] but issues such as +/// https://github.com/rust-lang/cargo/issues/12385 make their use impractical for the time being. +/// +/// This file implements an imperfect solution: it causes cargo to rebuild the crate whenever the +/// mtime of `which bpf-linker` changes. Note that possibility that a new bpf-linker is added to +/// $PATH ahead of the one used as the cache key still exists. Solving this in the general case +/// would require rebuild-if-changed-env=PATH *and* rebuild-if-changed={every-directory-in-PATH} +/// which would likely mean far too much cache invalidation. +/// +/// [bindeps]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html?highlight=feature#artifact-dependencies +fn main() { + println!("cargo:rerun-if-env-changed={}", AYA_BUILD_EBPF); + + let build_ebpf = env::var(AYA_BUILD_EBPF) + .as_deref() + .map(str::parse) + .map(Result::unwrap) + .unwrap_or_default(); + + if build_ebpf { + let bpf_linker = which("bpf-linker").unwrap(); + println!("cargo:rerun-if-changed={}", bpf_linker.to_str().unwrap()); + } +} diff --git a/{{project-name}}-ebpf/src/lib.rs b/{{project-name}}-ebpf/src/lib.rs new file mode 100644 index 0000000..3ac3e59 --- /dev/null +++ b/{{project-name}}-ebpf/src/lib.rs @@ -0,0 +1,3 @@ +#![no_std] + +// This file exists to enable the library target. diff --git a/{{project-name}}/Cargo.toml b/{{project-name}}/Cargo.toml index 83c71d2..23dd42c 100644 --- a/{{project-name}}/Cargo.toml +++ b/{{project-name}}/Cargo.toml @@ -18,6 +18,23 @@ tokio = { workspace = true, features = ["macros", "rt", "rt-multi-thread", "net" clap = { workspace = true, features = ["derive"] } {% endif -%} +[build-dependencies] +cargo_metadata = { workspace = true } +# TODO(https://github.com/rust-lang/cargo/issues/12375): this should be an artifact dependency, but +# it's not possible to tell cargo to use `-Z build-std` to build it. We cargo-in-cargo in the build +# script to build this, but we want to teach cargo about the dependecy so that cache invalidation +# works properly. +# +# Note also that https://github.com/rust-lang/cargo/issues/10593 occurs when `target = ...` is added +# to an artifact dependency; it seems possible to work around that by setting `resolver = "1"` in +# Cargo.toml in the workspace root. +# +# Finally note that *any* usage of `artifact = ...` in *any* Cargo.toml in the workspace breaks +# workflows with stable cargo; stable cargo outright refuses to load manifests that use unstable +# features. +{{project-name}}-ebpf = { path = "../{{project-name}}-ebpf" } +xtask = { path = "../xtask"} + [[bin]] name = "{{project-name}}" path = "src/main.rs" diff --git a/{{project-name}}/build.rs b/{{project-name}}/build.rs new file mode 100644 index 0000000..61caab3 --- /dev/null +++ b/{{project-name}}/build.rs @@ -0,0 +1,172 @@ +use std::{ + env, fs, + io::{BufRead as _, BufReader}, + path::PathBuf, + process::{Child, Command, Stdio}, +}; + +use cargo_metadata::{ + Artifact, CompilerMessage, Message, Metadata, MetadataCommand, Package, Target, +}; +use xtask::AYA_BUILD_EBPF; + +/// This crate has a runtime dependency on artifacts produced by the `{{project-name}}-ebpf` crate. +/// This would be better expressed as one or more [artifact-dependencies][bindeps] but issues such +/// as: +/// +/// * https://github.com/rust-lang/cargo/issues/12374 +/// * https://github.com/rust-lang/cargo/issues/12375 +/// * https://github.com/rust-lang/cargo/issues/12385 +/// +/// prevent their use for the time being. +/// +/// This file, along with the xtask crate, allows analysis tools such as `cargo check`, `cargo +/// clippy`, and even `cargo build` to work as users expect. Prior to this file's existence, this +/// crate's undeclared dependency on artifacts from `{{project-name}}-ebpf` would cause build (and +/// `cargo check`, and `cargo clippy`) failures until the user ran certain other commands in the +/// workspace. Conversely, those same tools (e.g. cargo test --no-run) would produce stale results +/// if run naively because they'd make use of artifacts from a previous build of +/// `{{project-name}}-ebpf`. +/// +/// Note that this solution is imperfect: in particular it has to balance correctness with +/// performance; an environment variable is used to replace true builds of `{{project-name}}-ebpf` +/// with stubs to preserve the property that code generation and linking (in +/// `{{project-name}}-ebpf`) do not occur on metadata-only actions such as `cargo check` or `cargo +/// clippy` of this crate. This means that naively attempting to `cargo test --no-run` this crate +/// will produce binaries that fail at runtime because the stubs are inadequate for actually running +/// the tests. +/// +/// [bindeps]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html?highlight=feature#artifact-dependencies +fn main() { + println!("cargo:rerun-if-env-changed={}", AYA_BUILD_EBPF); + + let build_ebpf = env::var(AYA_BUILD_EBPF) + .as_deref() + .map(str::parse) + .map(Result::unwrap) + .unwrap_or_default(); + + let Metadata { packages, .. } = MetadataCommand::new().no_deps().exec().unwrap(); + let ebpf_package = packages + .into_iter() + .find(|Package { name, .. }| name == "{{project-name}}-ebpf") + .unwrap(); + + let out_dir = env::var_os("OUT_DIR").unwrap(); + let out_dir = PathBuf::from(out_dir); + + let endian = env::var_os("CARGO_CFG_TARGET_ENDIAN").unwrap(); + let target = if endian == "big" { + "bpfeb" + } else if endian == "little" { + "bpfel" + } else { + panic!("unsupported endian={:?}", endian) + }; + + if build_ebpf { + let arch = env::var_os("CARGO_CFG_TARGET_ARCH").unwrap(); + + let target = format!("{target}-unknown-none"); + + let Package { manifest_path, .. } = ebpf_package; + let ebpf_dir = manifest_path.parent().unwrap(); + + // We have a build-dependency on `{{project-name}}-ebpf`, so cargo will automatically rebuild us + // if `{{project-name}}-ebpf`'s *library* target or any of its dependencies change. Since we + // depend on `{{project-name}}-ebpf`'s *binary* targets, that only gets us half of the way. This + // stanza ensures cargo will rebuild us on changes to the binaries too, which gets us the + // rest of the way. + println!("cargo:rerun-if-changed={}", ebpf_dir.as_str()); + + let mut cmd = Command::new("cargo"); + cmd.args([ + "build", + "-Z", + "build-std=core", + "--bins", + "--message-format=json", + "--release", + "--target", + &target, + ]); + + cmd.env("CARGO_CFG_BPF_TARGET_ARCH", arch); + + // Workaround to make sure that the rust-toolchain.toml is respected. + for key in ["RUSTUP_TOOLCHAIN", "RUSTC"] { + cmd.env_remove(key); + } + cmd.current_dir(ebpf_dir); + + // Workaround for https://github.com/rust-lang/cargo/issues/6412 where cargo flocks itself. + let ebpf_target_dir = out_dir.join("{{project-name}}-ebpf"); + cmd.arg("--target-dir").arg(&ebpf_target_dir); + + let mut child = cmd + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap_or_else(|err| panic!("failed to spawn {cmd:?}: {err}")); + let Child { stdout, stderr, .. } = &mut child; + + // Trampoline stdout to cargo warnings. + let stderr = stderr.take().unwrap(); + let stderr = BufReader::new(stderr); + let stderr = std::thread::spawn(move || { + for line in stderr.lines() { + let line = line.unwrap(); + println!("cargo:warning={line}"); + } + }); + + let stdout = stdout.take().unwrap(); + let stdout = BufReader::new(stdout); + let mut executables = Vec::new(); + for message in Message::parse_stream(stdout) { + #[allow(clippy::collapsible_match)] + match message.expect("valid JSON") { + Message::CompilerArtifact(Artifact { + executable, + target: Target { name, .. }, + .. + }) => { + if let Some(executable) = executable { + executables.push((name, executable.into_std_path_buf())); + } + } + Message::CompilerMessage(CompilerMessage { message, .. }) => { + for line in message.rendered.unwrap_or_default().split('\n') { + println!("cargo:warning={line}"); + } + } + Message::TextLine(line) => { + println!("cargo:warning={line}"); + } + _ => {} + } + } + + let status = child + .wait() + .unwrap_or_else(|err| panic!("failed to wait for {cmd:?}: {err}")); + assert_eq!(status.code(), Some(0), "{cmd:?} failed: {status:?}"); + + stderr.join().map_err(std::panic::resume_unwind).unwrap(); + + for (name, binary) in executables { + let dst = out_dir.join(name); + let _: u64 = fs::copy(&binary, &dst) + .unwrap_or_else(|err| panic!("failed to copy {binary:?} to {dst:?}: {err}")); + } + } else { + let Package { targets, .. } = ebpf_package; + for Target { name, kind, .. } in targets { + if *kind != ["bin"] { + continue; + } + let dst = out_dir.join(name); + fs::write(&dst, []).unwrap_or_else(|err| panic!("failed to create {dst:?}: {err}")); + } + } +} diff --git a/{{project-name}}/src/main.rs b/{{project-name}}/src/main.rs index 312bbd6..3e77a18 100644 --- a/{{project-name}}/src/main.rs +++ b/{{project-name}}/src/main.rs @@ -64,7 +64,7 @@ struct Opt { {% endif -%} #[tokio::main] -async fn main() -> Result<(), anyhow::Error> { +async fn main() -> anyhow::Result<()> { {%- if program_types_with_opts contains program_type %} let opt = Opt::parse(); {% endif %} @@ -85,14 +85,7 @@ async fn main() -> Result<(), anyhow::Error> { // runtime. This approach is recommended for most real-world use cases. If you would // like to specify the eBPF program at runtime rather than at compile-time, you can // reach for `Bpf::load_file` instead. - #[cfg(debug_assertions)] - let mut ebpf = Ebpf::load(include_bytes_aligned!( - "../../target/bpfel-unknown-none/debug/{{project-name}}" - ))?; - #[cfg(not(debug_assertions))] - let mut ebpf = Ebpf::load(include_bytes_aligned!( - "../../target/bpfel-unknown-none/release/{{project-name}}" - ))?; + let mut ebpf = Ebpf::load(include_bytes_aligned!(concat!(env!("OUT_DIR"), "/{{project-name}}")))?; if let Err(e) = EbpfLogger::init(&mut ebpf) { // This can happen if you remove all log statements from your eBPF program. warn!("failed to initialize eBPF logger: {}", e);