From 9916092f5c575ec2383c10fa45572d7dfc31aee3 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 4 Mar 2025 12:59:14 -0500 Subject: [PATCH] codegen: tidy up - Document the need for external rustfmt invocation. - Remove reexports. - Remove `write_to_file`. - Avoid allocating strings when using bindgen to write bindings. --- .github/workflows/gen.yml | 3 ++- aya-tool/src/generate.rs | 4 +-- aya-tool/src/lib.rs | 20 -------------- xtask/src/codegen/aya.rs | 19 +++++++------- xtask/src/codegen/aya_ebpf_bindings.rs | 36 +++++++++++++++----------- 5 files changed, 34 insertions(+), 48 deletions(-) diff --git a/.github/workflows/gen.yml b/.github/workflows/gen.yml index b50e71bf..b8696675 100644 --- a/.github/workflows/gen.yml +++ b/.github/workflows/gen.yml @@ -31,8 +31,9 @@ jobs: sudo apt -y install libelf-dev libc6-dev libc6-dev-{arm64,armel,loong64,riscv64,ppc64el,s390x,mips}-cross - run: cargo xtask codegen - - run: cargo xtask public-api --bless + # aya-ebpf-bindings aren't emitted directly from bindgen and so aren't formatted. - run: cargo fmt --all + - run: cargo xtask public-api --bless - id: libbpf working-directory: xtask/libbpf diff --git a/aya-tool/src/generate.rs b/aya-tool/src/generate.rs index 2d5b2a9c..eda2fa46 100644 --- a/aya-tool/src/generate.rs +++ b/aya-tool/src/generate.rs @@ -9,8 +9,6 @@ use std::{ use tempfile::tempdir; use thiserror::Error; -use crate::bindgen; - #[derive(Error, Debug)] pub enum Error { #[error("error executing bpftool")] @@ -44,7 +42,7 @@ pub fn generate>( .map(|s| s.as_ref().into()) .collect::>(); - let mut bindgen = bindgen::bpf_builder(); + let mut bindgen = crate::bindgen::bpf_builder(); let (additional_flags, ctypes_prefix) = extract_ctypes_prefix(&additional_flags); if let Some(prefix) = ctypes_prefix { diff --git a/aya-tool/src/lib.rs b/aya-tool/src/lib.rs index a132ac19..7b76595e 100644 --- a/aya-tool/src/lib.rs +++ b/aya-tool/src/lib.rs @@ -1,22 +1,2 @@ -use std::{ - fs::{File, create_dir_all}, - io::{self, Write}, - path::Path, -}; - pub mod bindgen; pub mod generate; - -pub use generate::{InputFile, generate}; - -pub fn write_to_file>(path: T, code: &str) -> Result<(), io::Error> { - // Create parent directories if they don't exist already - if let Some(parent) = path.as_ref().parent() { - if !parent.exists() { - create_dir_all(parent)?; - } - } - - let mut file = File::create(path)?; - file.write_all(code.as_bytes()) -} diff --git a/xtask/src/codegen/aya.rs b/xtask/src/codegen/aya.rs index dae7068e..47937501 100644 --- a/xtask/src/codegen/aya.rs +++ b/xtask/src/codegen/aya.rs @@ -1,7 +1,10 @@ -use std::path::{Path, PathBuf}; +use std::{ + fs::create_dir_all, + path::{Path, PathBuf}, +}; use anyhow::{Context as _, Result}; -use aya_tool::{bindgen, write_to_file}; +use aya_tool::bindgen; use crate::codegen::{Architecture, SysrootOptions}; @@ -26,10 +29,10 @@ fn codegen_internal_btf_bindings(libbpf_dir: &Path) -> Result<()> { bindgen = bindgen.allowlist_type(x); } - let bindings = bindgen.generate().context("bindgen failed")?.to_string(); + let bindings = bindgen.generate().context("bindgen failed")?; // write the bindings, with the original helpers removed - write_to_file(generated.join("btf_internal_bindings.rs"), &bindings)?; + bindings.write_to_file(generated.join("btf_internal_bindings.rs"))?; Ok(()) } @@ -47,6 +50,7 @@ fn codegen_bindings(opts: &SysrootOptions, libbpf_dir: &Path) -> Result<()> { } = opts; let dir = PathBuf::from("aya-obj"); let generated = dir.join("src/generated"); + create_dir_all(&generated)?; let builder = || { let mut bindgen = bindgen::user_builder() @@ -223,13 +227,10 @@ fn codegen_bindings(opts: &SysrootOptions, libbpf_dir: &Path) -> Result<()> { }; bindgen = bindgen.clang_args(["-I", sysroot.to_str().unwrap()]); - let bindings = bindgen.generate().context("bindgen failed")?.to_string(); + let bindings = bindgen.generate().context("bindgen failed")?; // write the bindings, with the original helpers removed - write_to_file( - generated.join(format!("linux_bindings_{arch}.rs")), - &bindings.to_string(), - )?; + bindings.write_to_file(generated.join(format!("linux_bindings_{arch}.rs")))?; } Ok(()) diff --git a/xtask/src/codegen/aya_ebpf_bindings.rs b/xtask/src/codegen/aya_ebpf_bindings.rs index fba8d5fd..e98ea695 100644 --- a/xtask/src/codegen/aya_ebpf_bindings.rs +++ b/xtask/src/codegen/aya_ebpf_bindings.rs @@ -1,12 +1,12 @@ use std::{ ffi::OsString, - fs::create_dir_all, + fs::{File, create_dir_all}, path::{Path, PathBuf}, process::Command, }; use anyhow::{Context as _, Result}; -use aya_tool::{bindgen, write_to_file}; +use aya_tool::bindgen; use proc_macro2::TokenStream; use quote::ToTokens; use syn::{Item, parse_str}; @@ -132,7 +132,8 @@ pub fn codegen(opts: &SysrootOptions, libbpf_dir: &Path) -> Result<()> { }; bindgen = bindgen.clang_args(["-I", sysroot.to_str().unwrap()]); - let bindings = bindgen.generate().context("bindgen failed")?.to_string(); + let bindings = bindgen.generate().context("bindgen failed")?; + let bindings = bindings.to_string(); let mut tree = parse_str::(&bindings).unwrap(); @@ -143,21 +144,26 @@ pub fn codegen(opts: &SysrootOptions, libbpf_dir: &Path) -> Result<()> { } let generated = dir.join("src").join(arch.to_string()); - if !generated.exists() { - create_dir_all(&generated)?; - } + create_dir_all(&generated)?; // write the bindings, with the original helpers removed - write_to_file( - generated.join("bindings.rs"), - &tree.to_token_stream().to_string(), - )?; - + // // write the new helpers as expanded by expand_helpers() - write_to_file( - generated.join("helpers.rs"), - &format!("use super::bindings::*; {helpers}"), - )?; + for (path, code) in [ + ( + generated.join("bindings.rs"), + &tree.to_token_stream().to_string(), + ), + ( + generated.join("helpers.rs"), + &format!("use super::bindings::*; {helpers}"), + ), + ] { + use std::io::Write as _; + + let mut file = File::create(path)?; + file.write_all(code.as_bytes())?; + } } Ok(())